diff --git a/container/container.go b/container/container.go index e89340610c..f22b4aa574 100644 --- a/container/container.go +++ b/container/container.go @@ -216,7 +216,7 @@ func (container *Container) WriteHostConfig() error { } // SetupWorkingDirectory sets up the container's working directory as set in container.Config.WorkingDir -func (container *Container) SetupWorkingDirectory(rootUID, rootGID int) error { +func (container *Container) SetupWorkingDirectory(rootIDs idtools.IDPair) error { if container.Config.WorkingDir == "" { return nil } @@ -228,7 +228,7 @@ func (container *Container) SetupWorkingDirectory(rootUID, rootGID int) error { return err } - if err := idtools.MkdirAllNewAs(pth, 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(pth, 0755, rootIDs); err != nil { pthInfo, err2 := os.Stat(pth) if err2 == nil && pthInfo != nil && !pthInfo.IsDir() { return fmt.Errorf("Cannot mkdir: %s is not a directory", container.Config.WorkingDir) diff --git a/container/container_unix.go b/container/container_unix.go index b46e100bb1..530343d5ca 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -425,7 +425,7 @@ func copyExistingContents(source, destination string) error { } if len(srcList) == 0 { // If the source volume is empty, copies files from the root into the volume - if err := chrootarchive.CopyWithTar(source, destination); err != nil { + if err := chrootarchive.NewArchiver(nil).CopyWithTar(source, destination); err != nil { return err } } diff --git a/daemon/archive.go b/daemon/archive.go index dbef0b4d48..3fb75bbb22 100644 --- a/daemon/archive.go +++ b/daemon/archive.go @@ -375,7 +375,7 @@ func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decomp destExists := true destDir := false - rootUID, rootGID := daemon.GetRemappedUIDGID() + rootIDs := daemon.idMappings.RootPair() // Work in daemon-local OS specific file paths destPath = filepath.FromSlash(destPath) @@ -413,13 +413,7 @@ func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decomp destExists = false } - uidMaps, gidMaps := daemon.GetUIDGIDMaps() - archiver := &archive.Archiver{ - Untar: chrootarchive.Untar, - UIDMaps: uidMaps, - GIDMaps: gidMaps, - } - + archiver := chrootarchive.NewArchiver(daemon.idMappings) src, err := os.Stat(fullSrcPath) if err != nil { return err @@ -430,7 +424,7 @@ func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decomp if err := archiver.CopyWithTar(fullSrcPath, destPath); err != nil { return err } - return fixPermissions(fullSrcPath, destPath, rootUID, rootGID, destExists) + return fixPermissions(fullSrcPath, destPath, rootIDs.UID, rootIDs.GID, destExists) } 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) @@ -459,12 +453,12 @@ func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decomp destPath = filepath.Join(destPath, filepath.Base(srcPath)) } - if err := idtools.MkdirAllNewAs(filepath.Dir(destPath), 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(filepath.Dir(destPath), 0755, rootIDs); err != nil { return err } if err := archiver.CopyFileWithTar(fullSrcPath, destPath); err != nil { return err } - return fixPermissions(fullSrcPath, destPath, rootUID, rootGID, destExists) + return fixPermissions(fullSrcPath, destPath, rootIDs.UID, rootIDs.GID, destExists) } diff --git a/daemon/archive_tarcopyoptions.go b/daemon/archive_tarcopyoptions.go index cbd1fc2cee..fe7722fdb4 100644 --- a/daemon/archive_tarcopyoptions.go +++ b/daemon/archive_tarcopyoptions.go @@ -7,10 +7,9 @@ import ( // defaultTarCopyOptions is the setting that is used when unpacking an archive // for a copy API event. func (daemon *Daemon) defaultTarCopyOptions(noOverwriteDirNonDir bool) *archive.TarOptions { - uidMaps, gidMaps := daemon.GetUIDGIDMaps() return &archive.TarOptions{ NoOverwriteDirNonDir: noOverwriteDirNonDir, - UIDMaps: uidMaps, - GIDMaps: gidMaps, + UIDMaps: daemon.idMappings.UIDs(), + GIDMaps: daemon.idMappings.GIDs(), } } diff --git a/daemon/archive_tarcopyoptions_unix.go b/daemon/archive_tarcopyoptions_unix.go index 7b68bc463d..83e6fd9e17 100644 --- a/daemon/archive_tarcopyoptions_unix.go +++ b/daemon/archive_tarcopyoptions_unix.go @@ -20,9 +20,6 @@ func (daemon *Daemon) tarCopyOptions(container *container.Container, noOverwrite return &archive.TarOptions{ NoOverwriteDirNonDir: noOverwriteDirNonDir, - ChownOpts: &archive.TarChownOptions{ - UID: user.Uid, - GID: user.Gid, - }, + ChownOpts: &idtools.IDPair{UID: user.Uid, GID: user.Gid}, }, nil } diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index 6889c10956..99d17e41b5 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -109,14 +109,14 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { } c.ShmPath = "/dev/shm" } else { - rootUID, rootGID := daemon.GetRemappedUIDGID() + rootIDs := daemon.idMappings.RootPair() if !c.HasMountFor("/dev/shm") { shmPath, err := c.ShmResourcePath() if err != nil { return err } - if err := idtools.MkdirAllAs(shmPath, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(shmPath, 0700, rootIDs); err != nil { return err } @@ -128,7 +128,7 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { if err := syscall.Mount("shm", shmPath, "tmpfs", uintptr(syscall.MS_NOEXEC|syscall.MS_NOSUID|syscall.MS_NODEV), label.FormatMountLabel(shmproperty, c.GetMountLabel())); err != nil { return fmt.Errorf("mounting shm tmpfs: %s", err) } - if err := os.Chown(shmPath, rootUID, rootGID); err != nil { + if err := os.Chown(shmPath, rootIDs.UID, rootIDs.GID); err != nil { return err } } @@ -147,9 +147,9 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { logrus.Debugf("secrets: setting up secret dir: %s", localMountPath) // retrieve possible remapped range start for root UID, GID - rootUID, rootGID := daemon.GetRemappedUIDGID() + rootIDs := daemon.idMappings.RootPair() // create tmpfs - if err := idtools.MkdirAllAs(localMountPath, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating secret local mount path") } @@ -164,7 +164,7 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { } }() - tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootUID, rootGID) + tmpfsOwnership := fmt.Sprintf("uid=%d,gid=%d", rootIDs.UID, rootIDs.GID) if err := mount.Mount("tmpfs", localMountPath, "tmpfs", "nodev,nosuid,noexec,"+tmpfsOwnership); err != nil { return errors.Wrap(err, "unable to setup secret mount") } @@ -183,7 +183,7 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { // secrets are created in the SecretMountPath on the host, at a // single level fPath := c.SecretFilePath(*s) - if err := idtools.MkdirAllAs(filepath.Dir(fPath), 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating secret mount path") } @@ -208,7 +208,7 @@ func (daemon *Daemon) setupSecretDir(c *container.Container) (setupErr error) { return err } - if err := os.Chown(fPath, rootUID+uid, rootGID+gid); err != nil { + if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil { return errors.Wrap(err, "error setting ownership for secret") } } @@ -232,9 +232,9 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { logrus.Debugf("configs: setting up config dir: %s", localPath) // retrieve possible remapped range start for root UID, GID - rootUID, rootGID := daemon.GetRemappedUIDGID() + rootIDs := daemon.idMappings.RootPair() // create tmpfs - if err := idtools.MkdirAllAs(localPath, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(localPath, 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config dir") } @@ -261,7 +261,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { log := logrus.WithFields(logrus.Fields{"name": configRef.File.Name, "path": fPath}) - if err := idtools.MkdirAllAs(filepath.Dir(fPath), 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(filepath.Dir(fPath), 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config path") } @@ -283,7 +283,7 @@ func (daemon *Daemon) setupConfigDir(c *container.Container) (setupErr error) { return err } - if err := os.Chown(fPath, rootUID+uid, rootGID+gid); err != nil { + if err := os.Chown(fPath, rootIDs.UID+uid, rootIDs.GID+gid); err != nil { return errors.Wrap(err, "error setting ownership for config") } } diff --git a/daemon/create.go b/daemon/create.go index de129ca60d..697886274f 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -117,14 +117,11 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( return nil, err } - rootUID, rootGID, err := idtools.GetRootUIDGID(daemon.uidMaps, daemon.gidMaps) - if err != nil { + rootIDs := daemon.idMappings.RootPair() + if err := idtools.MkdirAndChown(container.Root, 0700, rootIDs); err != nil { return nil, err } - if err := idtools.MkdirAs(container.Root, 0700, rootUID, rootGID); err != nil { - return nil, err - } - if err := idtools.MkdirAs(container.CheckpointDir(), 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAndChown(container.CheckpointDir(), 0700, rootIDs); err != nil { return nil, err } diff --git a/daemon/create_unix.go b/daemon/create_unix.go index 7b067bdc22..2501a3374a 100644 --- a/daemon/create_unix.go +++ b/daemon/create_unix.go @@ -22,8 +22,8 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain } defer daemon.Unmount(container) - rootUID, rootGID := daemon.GetRemappedUIDGID() - if err := container.SetupWorkingDirectory(rootUID, rootGID); err != nil { + rootIDs := daemon.idMappings.RootPair() + if err := container.SetupWorkingDirectory(rootIDs); err != nil { return err } diff --git a/daemon/daemon.go b/daemon/daemon.go index 05017775d4..4b78d01370 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -94,8 +94,7 @@ type Daemon struct { seccompEnabled bool apparmorEnabled bool shutdown bool - uidMaps []idtools.IDMap - gidMaps []idtools.IDMap + idMappings *idtools.IDMappings layerStore layer.Store imageStore image.Store PluginStore *plugin.Store // todo: remove @@ -524,21 +523,17 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe return nil, err } - uidMaps, gidMaps, err := setupRemappedRoot(config) + idMappings, err := setupRemappedRoot(config) if err != nil { return nil, err } - rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps) - if err != nil { - return nil, err - } - + rootIDs := idMappings.RootPair() if err := setupDaemonProcess(config); err != nil { return nil, err } // set up the tmpDir to use a canonical path - tmp, err := prepareTempDir(config.Root, rootUID, rootGID) + tmp, err := prepareTempDir(config.Root, rootIDs) if err != nil { return nil, fmt.Errorf("Unable to get the TempDir under %s: %s", config.Root, err) } @@ -587,7 +582,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe } daemonRepo := filepath.Join(config.Root, "containers") - if err := idtools.MkdirAllAs(daemonRepo, 0700, rootUID, rootGID); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAllAndChown(daemonRepo, 0700, rootIDs); err != nil && !os.IsExist(err) { return nil, err } @@ -632,8 +627,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe MetadataStorePathTemplate: filepath.Join(config.Root, "image", "%s", "layerdb"), GraphDriver: driverName, GraphDriverOptions: config.GraphOptions, - UIDMaps: uidMaps, - GIDMaps: gidMaps, + IDMappings: idMappings, PluginGetter: d.PluginStore, ExperimentalEnabled: config.Experimental, }) @@ -665,7 +659,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe } // Configure the volumes driver - volStore, err := d.configureVolumes(rootUID, rootGID) + volStore, err := d.configureVolumes(rootIDs) if err != nil { return nil, err } @@ -728,8 +722,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe d.EventsService = eventsService d.volumes = volStore d.root = config.Root - d.uidMaps = uidMaps - d.gidMaps = gidMaps + d.idMappings = idMappings d.seccompEnabled = sysInfo.Seccomp d.apparmorEnabled = sysInfo.AppArmor @@ -970,25 +963,10 @@ func (daemon *Daemon) GraphDriverName() string { return daemon.layerStore.DriverName() } -// GetUIDGIDMaps returns the current daemon's user namespace settings -// for the full uid and gid maps which will be applied to containers -// started in this instance. -func (daemon *Daemon) GetUIDGIDMaps() ([]idtools.IDMap, []idtools.IDMap) { - return daemon.uidMaps, daemon.gidMaps -} - -// GetRemappedUIDGID returns the current daemon's uid and gid values -// if user namespaces are in use for this daemon instance. If not -// this function will return "real" root values of 0, 0. -func (daemon *Daemon) GetRemappedUIDGID() (int, int) { - uid, gid, _ := idtools.GetRootUIDGID(daemon.uidMaps, daemon.gidMaps) - return uid, gid -} - // prepareTempDir prepares and returns the default directory to use // for temporary files. // If it doesn't exist, it is created. If it exists, its content is removed. -func prepareTempDir(rootDir string, rootUID, rootGID int) (string, error) { +func prepareTempDir(rootDir string, rootIDs idtools.IDPair) (string, error) { var tmpDir string if tmpDir = os.Getenv("DOCKER_TMPDIR"); tmpDir == "" { tmpDir = filepath.Join(rootDir, "tmp") @@ -1008,12 +986,12 @@ func prepareTempDir(rootDir string, rootUID, rootGID int) (string, error) { } // We don't remove the content of tmpdir if it's not the default, // it may hold things that do not belong to us. - return tmpDir, idtools.MkdirAllAs(tmpDir, 0700, rootUID, rootGID) + return tmpDir, idtools.MkdirAllAndChown(tmpDir, 0700, rootIDs) } func (daemon *Daemon) setupInitLayer(initPath string) error { - rootUID, rootGID := daemon.GetRemappedUIDGID() - return initlayer.Setup(initPath, rootUID, rootGID) + rootIDs := daemon.idMappings.RootPair() + return initlayer.Setup(initPath, rootIDs) } func setDefaultMtu(conf *config.Config) { @@ -1024,8 +1002,8 @@ func setDefaultMtu(conf *config.Config) { conf.Mtu = config.DefaultNetworkMtu } -func (daemon *Daemon) configureVolumes(rootUID, rootGID int) (*store.VolumeStore, error) { - volumesDriver, err := local.New(daemon.configStore.Root, rootUID, rootGID) +func (daemon *Daemon) configureVolumes(rootIDs idtools.IDPair) (*store.VolumeStore, error) { + volumesDriver, err := local.New(daemon.configStore.Root, rootIDs) if err != nil { return nil, err } @@ -1171,18 +1149,9 @@ func CreateDaemonRoot(config *config.Config) error { } } - uidMaps, gidMaps, err := setupRemappedRoot(config) + idMappings, err := setupRemappedRoot(config) if err != nil { return err } - rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps) - if err != nil { - return err - } - - if err := setupDaemonRoot(config, realRoot, rootUID, rootGID); err != nil { - return err - } - - return nil + return setupDaemonRoot(config, realRoot, idMappings.RootPair()) } diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index eaa3be44d8..e728abddf8 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -11,6 +11,7 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/container" _ "github.com/docker/docker/pkg/discovery/memory" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/registrar" "github.com/docker/docker/pkg/truncindex" "github.com/docker/docker/volume" @@ -127,7 +128,7 @@ func initDaemonWithVolumeStore(tmp string) (*Daemon, error) { return nil, err } - volumesDriver, err := local.New(tmp, 0, 0) + volumesDriver, err := local.New(tmp, idtools.IDPair{UID: 0, GID: 0}) if err != nil { return nil, err } diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 031cdc4a8b..07c323d63f 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -1026,40 +1026,38 @@ func parseRemappedRoot(usergrp string) (string, string, error) { return username, groupname, nil } -func setupRemappedRoot(config *config.Config) ([]idtools.IDMap, []idtools.IDMap, error) { +func setupRemappedRoot(config *config.Config) (*idtools.IDMappings, error) { if runtime.GOOS != "linux" && config.RemappedRoot != "" { - return nil, nil, fmt.Errorf("User namespaces are only supported on Linux") + return nil, fmt.Errorf("User namespaces are only supported on Linux") } // if the daemon was started with remapped root option, parse // the config option to the int uid,gid values - var ( - uidMaps, gidMaps []idtools.IDMap - ) if config.RemappedRoot != "" { username, groupname, err := parseRemappedRoot(config.RemappedRoot) if err != nil { - return nil, nil, err + return nil, err } if username == "root" { // Cannot setup user namespaces with a 1-to-1 mapping; "--root=0:0" is a no-op // effectively logrus.Warn("User namespaces: root cannot be remapped with itself; user namespaces are OFF") - return uidMaps, gidMaps, nil + return &idtools.IDMappings{}, nil } logrus.Infof("User namespaces: ID ranges will be mapped to subuid/subgid ranges of: %s:%s", username, groupname) // update remapped root setting now that we have resolved them to actual names config.RemappedRoot = fmt.Sprintf("%s:%s", username, groupname) - uidMaps, gidMaps, err = idtools.CreateIDMappings(username, groupname) + mappings, err := idtools.NewIDMappings(username, groupname) if err != nil { - return nil, nil, fmt.Errorf("Can't create ID mappings: %v", err) + return nil, errors.Wrapf(err, "Can't create ID mappings: %v") } + return mappings, nil } - return uidMaps, gidMaps, nil + return &idtools.IDMappings{}, nil } -func setupDaemonRoot(config *config.Config, rootDir string, rootUID, rootGID int) error { +func setupDaemonRoot(config *config.Config, rootDir string, rootIDs idtools.IDPair) error { config.Root = rootDir // the docker root metadata directory needs to have execute permissions for all users (g+x,o+x) // so that syscalls executing as non-root, operating on subdirectories of the graph root @@ -1084,10 +1082,10 @@ func setupDaemonRoot(config *config.Config, rootDir string, rootUID, rootGID int // a new subdirectory with ownership set to the remapped uid/gid (so as to allow // `chdir()` to work for containers namespaced to that uid/gid) if config.RemappedRoot != "" { - config.Root = filepath.Join(rootDir, fmt.Sprintf("%d.%d", rootUID, rootGID)) + config.Root = filepath.Join(rootDir, fmt.Sprintf("%d.%d", rootIDs.UID, rootIDs.GID)) logrus.Debugf("Creating user namespaced daemon root: %s", config.Root) // Create the root directory if it doesn't exist - if err := idtools.MkdirAllAs(config.Root, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(config.Root, 0700, rootIDs); err != nil { return fmt.Errorf("Cannot create daemon root: %s: %v", config.Root, err) } // we also need to verify that any pre-existing directories in the path to @@ -1100,7 +1098,7 @@ func setupDaemonRoot(config *config.Config, rootDir string, rootUID, rootGID int if dirPath == "/" { break } - if !idtools.CanAccess(dirPath, rootUID, rootGID) { + if !idtools.CanAccess(dirPath, rootIDs) { return fmt.Errorf("A subdirectory in your graphroot path (%s) restricts access to the remapped root uid/gid; please fix by allowing 'o+x' permissions on existing directories.", config.Root) } } diff --git a/daemon/daemon_unix_test.go b/daemon/daemon_unix_test.go index e8afe629e0..a2847bfbee 100644 --- a/daemon/daemon_unix_test.go +++ b/daemon/daemon_unix_test.go @@ -11,6 +11,7 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/container" "github.com/docker/docker/daemon/config" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/volume" "github.com/docker/docker/volume/drivers" "github.com/docker/docker/volume/local" @@ -277,7 +278,7 @@ func TestMigratePre17Volumes(t *testing.T) { if err != nil { t.Fatal(err) } - drv, err := local.New(volumeRoot, 0, 0) + drv, err := local.New(volumeRoot, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index efe1b1f6a0..aced740df3 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -454,11 +454,11 @@ func (daemon *Daemon) cleanupMounts() error { return nil } -func setupRemappedRoot(config *config.Config) ([]idtools.IDMap, []idtools.IDMap, error) { - return nil, nil, nil +func setupRemappedRoot(config *config.Config) (*idtools.IDMappings, error) { + return &idtools.IDMappings{}, nil } -func setupDaemonRoot(config *config.Config, rootDir string, rootUID, rootGID int) error { +func setupDaemonRoot(config *config.Config, rootDir string, rootIDs idtools.IDPair) error { config.Root = rootDir // Create the root directory if it doesn't exists if err := system.MkdirAllWithACL(config.Root, 0); err != nil && !os.IsExist(err) { diff --git a/daemon/export.go b/daemon/export.go index 5ef6dbb0e5..402e67583d 100644 --- a/daemon/export.go +++ b/daemon/export.go @@ -40,11 +40,10 @@ func (daemon *Daemon) containerExport(container *container.Container) (io.ReadCl return nil, err } - uidMaps, gidMaps := daemon.GetUIDGIDMaps() archive, err := archive.TarWithOptions(container.BaseFS, &archive.TarOptions{ Compression: archive.Uncompressed, - UIDMaps: uidMaps, - GIDMaps: gidMaps, + UIDMaps: daemon.idMappings.UIDs(), + GIDMaps: daemon.idMappings.GIDs(), }) if err != nil { daemon.Unmount(container) diff --git a/daemon/graphdriver/devmapper/devmapper_test.go b/daemon/graphdriver/devmapper/devmapper_test.go index c5be97ae3f..7501397fdc 100644 --- a/daemon/graphdriver/devmapper/devmapper_test.go +++ b/daemon/graphdriver/devmapper/devmapper_test.go @@ -4,6 +4,8 @@ package devmapper import ( "fmt" + "os" + "syscall" "testing" "time" @@ -17,11 +19,51 @@ func init() { defaultMetaDataLoopbackSize = 200 * 1024 * 1024 defaultBaseFsSize = 300 * 1024 * 1024 defaultUdevSyncOverride = true - if err := graphtest.InitLoopbacks(); err != nil { + if err := initLoopbacks(); err != nil { panic(err) } } +// initLoopbacks ensures that the loopback devices are properly created within +// the system running the device mapper tests. +func initLoopbacks() error { + statT, err := getBaseLoopStats() + if err != nil { + return err + } + // create at least 8 loopback files, ya, that is a good number + for i := 0; i < 8; i++ { + loopPath := fmt.Sprintf("/dev/loop%d", i) + // only create new loopback files if they don't exist + if _, err := os.Stat(loopPath); err != nil { + if mkerr := syscall.Mknod(loopPath, + uint32(statT.Mode|syscall.S_IFBLK), int((7<<8)|(i&0xff)|((i&0xfff00)<<12))); mkerr != nil { + return mkerr + } + os.Chown(loopPath, int(statT.Uid), int(statT.Gid)) + } + } + return nil +} + +// getBaseLoopStats inspects /dev/loop0 to collect uid,gid, and mode for the +// loop0 device on the system. If it does not exist we assume 0,0,0660 for the +// stat data +func getBaseLoopStats() (*syscall.Stat_t, error) { + loop0, err := os.Stat("/dev/loop0") + if err != nil { + if os.IsNotExist(err) { + return &syscall.Stat_t{ + Uid: 0, + Gid: 0, + Mode: 0660, + }, nil + } + return nil, err + } + return loop0.Sys().(*syscall.Stat_t), nil +} + // This avoids creating a new driver for each test if all tests are run // Make sure to put new tests between TestDevmapperSetup and TestDevmapperTeardown func TestDevmapperSetup(t *testing.T) { diff --git a/daemon/graphdriver/graphtest/graphtest_unix.go b/daemon/graphdriver/graphtest/graphtest_unix.go index 6e952de78b..6852ca9f4c 100644 --- a/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/daemon/graphdriver/graphtest/graphtest_unix.go @@ -16,6 +16,8 @@ import ( "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/stringid" "github.com/docker/go-units" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -33,14 +35,9 @@ type Driver struct { func newDriver(t testing.TB, name string, options []string) *Driver { root, err := ioutil.TempDir("", "docker-graphtest-") - if err != nil { - t.Fatal(err) - } - - if err := os.MkdirAll(root, 0755); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, os.MkdirAll(root, 0755)) d, err := graphdriver.GetDriver(name, nil, graphdriver.Options{DriverOptions: options, Root: root}) if err != nil { t.Logf("graphdriver: %v\n", err) @@ -86,14 +83,11 @@ func DriverTestCreateEmpty(t testing.TB, drivername string, driverOptions ...str driver := GetDriver(t, drivername, driverOptions...) defer PutDriver(t) - if err := driver.Create("empty", "", nil); err != nil { - t.Fatal(err) - } + err := driver.Create("empty", "", nil) + require.NoError(t, err) defer func() { - if err := driver.Remove("empty"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("empty")) }() if !driver.Exists("empty") { @@ -101,21 +95,14 @@ func DriverTestCreateEmpty(t testing.TB, drivername string, driverOptions ...str } dir, err := driver.Get("empty", "") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) verifyFile(t, dir, 0755|os.ModeDir, 0, 0) // Verify that the directory is empty fis, err := readDir(dir) - if err != nil { - t.Fatal(err) - } - - if len(fis) != 0 { - t.Fatal("New directory not empty") - } + require.NoError(t, err) + assert.Len(t, fis, 0) driver.Put("empty") } @@ -127,9 +114,7 @@ func DriverTestCreateBase(t testing.TB, drivername string, driverOptions ...stri createBase(t, driver, "Base") defer func() { - if err := driver.Remove("Base"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("Base")) }() verifyBase(t, driver, "Base") } @@ -140,21 +125,14 @@ func DriverTestCreateSnap(t testing.TB, drivername string, driverOptions ...stri defer PutDriver(t) createBase(t, driver, "Base") - defer func() { - if err := driver.Remove("Base"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("Base")) }() - if err := driver.Create("Snap", "Base", nil); err != nil { - t.Fatal(err) - } - + err := driver.Create("Snap", "Base", nil) + require.NoError(t, err) defer func() { - if err := driver.Remove("Snap"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("Snap")) }() verifyBase(t, driver, "Snap") diff --git a/daemon/graphdriver/graphtest/testutil_unix.go b/daemon/graphdriver/graphtest/testutil_unix.go index 49b0c2cc35..63a934176e 100644 --- a/daemon/graphdriver/graphtest/testutil_unix.go +++ b/daemon/graphdriver/graphtest/testutil_unix.go @@ -3,7 +3,6 @@ package graphtest import ( - "fmt" "io/ioutil" "os" "path" @@ -11,81 +10,24 @@ import ( "testing" "github.com/docker/docker/daemon/graphdriver" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -// InitLoopbacks ensures that the loopback devices are properly created within -// the system running the device mapper tests. -func InitLoopbacks() error { - statT, err := getBaseLoopStats() - if err != nil { - return err - } - // create at least 8 loopback files, ya, that is a good number - for i := 0; i < 8; i++ { - loopPath := fmt.Sprintf("/dev/loop%d", i) - // only create new loopback files if they don't exist - if _, err := os.Stat(loopPath); err != nil { - if mkerr := syscall.Mknod(loopPath, - uint32(statT.Mode|syscall.S_IFBLK), int((7<<8)|(i&0xff)|((i&0xfff00)<<12))); mkerr != nil { - return mkerr - } - os.Chown(loopPath, int(statT.Uid), int(statT.Gid)) - } - } - return nil -} - -// getBaseLoopStats inspects /dev/loop0 to collect uid,gid, and mode for the -// loop0 device on the system. If it does not exist we assume 0,0,0660 for the -// stat data -func getBaseLoopStats() (*syscall.Stat_t, error) { - loop0, err := os.Stat("/dev/loop0") - if err != nil { - if os.IsNotExist(err) { - return &syscall.Stat_t{ - Uid: 0, - Gid: 0, - Mode: 0660, - }, nil - } - return nil, err - } - return loop0.Sys().(*syscall.Stat_t), nil -} - func verifyFile(t testing.TB, path string, mode os.FileMode, uid, gid uint32) { fi, err := os.Stat(path) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if fi.Mode()&os.ModeType != mode&os.ModeType { - t.Fatalf("Expected %s type 0x%x, got 0x%x", path, mode&os.ModeType, fi.Mode()&os.ModeType) - } - - if fi.Mode()&os.ModePerm != mode&os.ModePerm { - t.Fatalf("Expected %s mode %o, got %o", path, mode&os.ModePerm, fi.Mode()&os.ModePerm) - } - - if fi.Mode()&os.ModeSticky != mode&os.ModeSticky { - t.Fatalf("Expected %s sticky 0x%x, got 0x%x", path, mode&os.ModeSticky, fi.Mode()&os.ModeSticky) - } - - if fi.Mode()&os.ModeSetuid != mode&os.ModeSetuid { - t.Fatalf("Expected %s setuid 0x%x, got 0x%x", path, mode&os.ModeSetuid, fi.Mode()&os.ModeSetuid) - } - - if fi.Mode()&os.ModeSetgid != mode&os.ModeSetgid { - t.Fatalf("Expected %s setgid 0x%x, got 0x%x", path, mode&os.ModeSetgid, fi.Mode()&os.ModeSetgid) - } + actual := fi.Mode() + assert.Equal(t, mode&os.ModeType, actual&os.ModeType, path) + assert.Equal(t, mode&os.ModePerm, actual&os.ModePerm, path) + assert.Equal(t, mode&os.ModeSticky, actual&os.ModeSticky, path) + assert.Equal(t, mode&os.ModeSetuid, actual&os.ModeSetuid, path) + assert.Equal(t, mode&os.ModeSetgid, actual&os.ModeSetgid, path) if stat, ok := fi.Sys().(*syscall.Stat_t); ok { - if stat.Uid != uid { - t.Fatalf("%s no owned by uid %d", path, uid) - } - if stat.Gid != gid { - t.Fatalf("%s not owned by gid %d", path, gid) - } + assert.Equal(t, uid, stat.Uid, path) + assert.Equal(t, gid, stat.Gid, path) } } @@ -94,35 +36,25 @@ func createBase(t testing.TB, driver graphdriver.Driver, name string) { oldmask := syscall.Umask(0) defer syscall.Umask(oldmask) - if err := driver.CreateReadWrite(name, "", nil); err != nil { - t.Fatal(err) - } + err := driver.CreateReadWrite(name, "", nil) + require.NoError(t, err) dir, err := driver.Get(name, "") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer driver.Put(name) subdir := path.Join(dir, "a subdir") - if err := os.Mkdir(subdir, 0705|os.ModeSticky); err != nil { - t.Fatal(err) - } - if err := os.Chown(subdir, 1, 2); err != nil { - t.Fatal(err) - } + require.NoError(t, os.Mkdir(subdir, 0705|os.ModeSticky)) + require.NoError(t, os.Chown(subdir, 1, 2)) file := path.Join(dir, "a file") - if err := ioutil.WriteFile(file, []byte("Some data"), 0222|os.ModeSetuid); err != nil { - t.Fatal(err) - } + err = ioutil.WriteFile(file, []byte("Some data"), 0222|os.ModeSetuid) + require.NoError(t, err) } func verifyBase(t testing.TB, driver graphdriver.Driver, name string) { dir, err := driver.Get(name, "") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer driver.Put(name) subdir := path.Join(dir, "a subdir") @@ -131,13 +63,7 @@ func verifyBase(t testing.TB, driver graphdriver.Driver, name string) { file := path.Join(dir, "a file") verifyFile(t, file, 0222|os.ModeSetuid, 0, 0) - fis, err := readDir(dir) - if err != nil { - t.Fatal(err) - } - - if len(fis) != 2 { - t.Fatal("Unexpected files in base image") - } - + files, err := readDir(dir) + require.NoError(t, err) + assert.Len(t, files, 2) } diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 846215e810..15a4de3606 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -9,13 +9,12 @@ import ( "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/system" - "github.com/opencontainers/selinux/go-selinux/label" ) var ( // CopyWithTar defines the copy method to use. - CopyWithTar = chrootarchive.CopyWithTar + CopyWithTar = chrootarchive.NewArchiver(nil).CopyWithTar ) func init() { @@ -26,15 +25,11 @@ func init() { // This sets the home directory for the driver and returns NaiveDiffDriver. func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (graphdriver.Driver, error) { d := &Driver{ - home: home, - uidMaps: uidMaps, - gidMaps: gidMaps, + home: home, + idMappings: idtools.NewIDMappingsFromMaps(uidMaps, gidMaps), } - rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps) - if err != nil { - return nil, err - } - if err := idtools.MkdirAllAs(home, 0700, rootUID, rootGID); err != nil { + rootIDs := d.idMappings.RootPair() + if err := idtools.MkdirAllAndChown(home, 0700, rootIDs); err != nil { return nil, err } return graphdriver.NewNaiveDiffDriver(d, uidMaps, gidMaps), nil @@ -45,9 +40,8 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap // In order to support layering, files are copied from the parent layer into the new layer. There is no copy-on-write support. // Driver must be wrapped in NaiveDiffDriver to be used as a graphdriver.Driver type Driver struct { - home string - uidMaps []idtools.IDMap - gidMaps []idtools.IDMap + home string + idMappings *idtools.IDMappings } func (d *Driver) String() string { @@ -82,14 +76,11 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error { } dir := d.dir(id) - rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) - if err != nil { + rootIDs := d.idMappings.RootPair() + if err := idtools.MkdirAllAndChown(filepath.Dir(dir), 0700, rootIDs); err != nil { return err } - if err := idtools.MkdirAllAs(filepath.Dir(dir), 0700, rootUID, rootGID); err != nil { - return err - } - if err := idtools.MkdirAs(dir, 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAndChown(dir, 0755, rootIDs); err != nil { return err } labelOpts := []string{"level:s0"} @@ -103,10 +94,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error { if err != nil { return fmt.Errorf("%s: %s", parent, err) } - if err := CopyWithTar(parentDir, dir); err != nil { - return err - } - return nil + return CopyWithTar(parentDir, dir) } func (d *Driver) dir(id string) string { diff --git a/daemon/info.go b/daemon/info.go index 2be4b395fa..3c28cdf7f1 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -72,8 +72,8 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { if selinuxEnabled() { securityOptions = append(securityOptions, "name=selinux") } - uid, gid := daemon.GetRemappedUIDGID() - if uid != 0 || gid != 0 { + rootIDs := daemon.idMappings.RootPair() + if rootIDs.UID != 0 || rootIDs.GID != 0 { securityOptions = append(securityOptions, "name=userns") } diff --git a/daemon/initlayer/setup_unix.go b/daemon/initlayer/setup_unix.go index e83c2751ed..cdd8973481 100644 --- a/daemon/initlayer/setup_unix.go +++ b/daemon/initlayer/setup_unix.go @@ -16,7 +16,7 @@ import ( // // This extra layer is used by all containers as the top-most ro layer. It protects // the container from unwanted side-effects on the rw layer. -func Setup(initLayer string, rootUID, rootGID int) error { +func Setup(initLayer string, rootIDs idtools.IDPair) error { for pth, typ := range map[string]string{ "/dev/pts": "dir", "/dev/shm": "dir", @@ -38,12 +38,12 @@ func Setup(initLayer string, rootUID, rootGID int) error { if _, err := os.Stat(filepath.Join(initLayer, pth)); err != nil { if os.IsNotExist(err) { - if err := idtools.MkdirAllNewAs(filepath.Join(initLayer, filepath.Dir(pth)), 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(filepath.Join(initLayer, filepath.Dir(pth)), 0755, rootIDs); err != nil { return err } switch typ { case "dir": - if err := idtools.MkdirAllNewAs(filepath.Join(initLayer, pth), 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(filepath.Join(initLayer, pth), 0755, rootIDs); err != nil { return err } case "file": @@ -51,7 +51,7 @@ func Setup(initLayer string, rootUID, rootGID int) error { if err != nil { return err } - f.Chown(rootUID, rootGID) + f.Chown(rootIDs.UID, rootIDs.GID) f.Close() default: if err := os.Symlink(typ, filepath.Join(initLayer, pth)); err != nil { diff --git a/daemon/initlayer/setup_windows.go b/daemon/initlayer/setup_windows.go index 48a9d71aa5..2b22f58b5e 100644 --- a/daemon/initlayer/setup_windows.go +++ b/daemon/initlayer/setup_windows.go @@ -2,12 +2,16 @@ package initlayer +import ( + "github.com/docker/docker/pkg/idtools" +) + // Setup populates a directory with mountpoints suitable // for bind-mounting dockerinit into the container. The mountpoint is simply an // empty file at /.dockerinit // // This extra layer is used by all containers as the top-most ro layer. It protects // the container from unwanted side-effects on the rw layer. -func Setup(initLayer string, rootUID, rootGID int) error { +func Setup(initLayer string, rootIDs idtools.IDPair) error { return nil } diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index 55a6cd8ae0..6d74301a05 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -271,13 +271,13 @@ func setNamespaces(daemon *Daemon, s *specs.Spec, c *container.Container) error userNS := false // user if c.HostConfig.UsernsMode.IsPrivate() { - uidMap, gidMap := daemon.GetUIDGIDMaps() + uidMap := daemon.idMappings.UIDs() if uidMap != nil { userNS = true ns := specs.LinuxNamespace{Type: "user"} setNamespace(s, ns) s.Linux.UIDMappings = specMapping(uidMap) - s.Linux.GIDMappings = specMapping(gidMap) + s.Linux.GIDMappings = specMapping(daemon.idMappings.GIDs()) } } // network @@ -591,7 +591,7 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c // TODO: until a kernel/mount solution exists for handling remount in a user namespace, // we must clear the readonly flag for the cgroups mount (@mrunalp concurs) - if uidMap, _ := daemon.GetUIDGIDMaps(); uidMap != nil || c.HostConfig.Privileged { + if uidMap := daemon.idMappings.UIDs(); uidMap != nil || c.HostConfig.Privileged { for i, m := range s.Mounts { if m.Type == "cgroup" { clearReadOnly(&s.Mounts[i]) @@ -611,8 +611,7 @@ func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) Path: c.BaseFS, Readonly: c.HostConfig.ReadonlyRootfs, } - rootUID, rootGID := daemon.GetRemappedUIDGID() - if err := c.SetupWorkingDirectory(rootUID, rootGID); err != nil { + if err := c.SetupWorkingDirectory(daemon.idMappings.RootPair()); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/daemon/oci_solaris.go b/daemon/oci_solaris.go index 0c757f9196..610efe10a1 100644 --- a/daemon/oci_solaris.go +++ b/daemon/oci_solaris.go @@ -130,8 +130,7 @@ func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) Path: filepath.Dir(c.BaseFS), Readonly: c.HostConfig.ReadonlyRootfs, } - rootUID, rootGID := daemon.GetRemappedUIDGID() - if err := c.SetupWorkingDirectory(rootUID, rootGID); err != nil { + if err := c.SetupWorkingDirectory(daemon.idMappings.RootPair()); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index 370eb74218..d91100f1c9 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -54,8 +54,7 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er return nil } - rootUID, rootGID := daemon.GetRemappedUIDGID() - path, err := m.Setup(c.MountLabel, rootUID, rootGID, checkfunc) + path, err := m.Setup(c.MountLabel, daemon.idMappings.RootPair(), checkfunc) if err != nil { return nil, err } @@ -85,9 +84,9 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er // if we are going to mount any of the network files from container // metadata, the ownership must be set properly for potential container // remapped root (user namespaces) - rootUID, rootGID := daemon.GetRemappedUIDGID() + rootIDs := daemon.idMappings.RootPair() for _, mount := range netMounts { - if err := os.Chown(mount.Source, rootUID, rootGID); err != nil { + if err := os.Chown(mount.Source, rootIDs.UID, rootIDs.GID); err != nil { return nil, err } } diff --git a/daemon/volumes_windows.go b/daemon/volumes_windows.go index 76940c67df..62c9e23ac0 100644 --- a/daemon/volumes_windows.go +++ b/daemon/volumes_windows.go @@ -6,6 +6,7 @@ import ( "sort" "github.com/docker/docker/container" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/volume" ) @@ -24,7 +25,7 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er if err := daemon.lazyInitializeVolume(c.ID, mount); err != nil { return nil, err } - s, err := mount.Setup(c.MountLabel, 0, 0, nil) + s, err := mount.Setup(c.MountLabel, idtools.IDPair{0, 0}, nil) if err != nil { return nil, err } diff --git a/daemon/workdir.go b/daemon/workdir.go index 99a2a8ea57..6360f24138 100644 --- a/daemon/workdir.go +++ b/daemon/workdir.go @@ -16,6 +16,5 @@ func (daemon *Daemon) ContainerCreateWorkdir(cID string) error { return err } defer daemon.Unmount(container) - rootUID, rootGID := daemon.GetRemappedUIDGID() - return container.SetupWorkingDirectory(rootUID, rootGID) + return container.SetupWorkingDirectory(daemon.idMappings.RootPair()) } diff --git a/distribution/registry_unit_test.go b/distribution/registry_unit_test.go index ebe6ecad97..910061f45a 100644 --- a/distribution/registry_unit_test.go +++ b/distribution/registry_unit_test.go @@ -152,7 +152,7 @@ func testDirectory(templateDir string) (dir string, err error) { return } if templateDir != "" { - if err = archive.CopyWithTar(templateDir, dir); err != nil { + if err = archive.NewDefaultArchiver().CopyWithTar(templateDir, dir); err != nil { return } } diff --git a/layer/layer_store.go b/layer/layer_store.go index 5caa5d41f2..25861c6669 100644 --- a/layer/layer_store.go +++ b/layer/layer_store.go @@ -44,8 +44,7 @@ type StoreOptions struct { MetadataStorePathTemplate string GraphDriver string GraphDriverOptions []string - UIDMaps []idtools.IDMap - GIDMaps []idtools.IDMap + IDMappings *idtools.IDMappings PluginGetter plugingetter.PluginGetter ExperimentalEnabled bool } @@ -55,8 +54,8 @@ func NewStoreFromOptions(options StoreOptions) (Store, error) { driver, err := graphdriver.New(options.GraphDriver, options.PluginGetter, graphdriver.Options{ Root: options.StorePath, DriverOptions: options.GraphDriverOptions, - UIDMaps: options.UIDMaps, - GIDMaps: options.GIDMaps, + UIDMaps: options.IDMappings.UIDs(), + GIDMaps: options.IDMappings.GIDs(), ExperimentalEnabled: options.ExperimentalEnabled, }) if err != nil { diff --git a/layer/layer_test.go b/layer/layer_test.go index 5c4da4f9a9..56340d3012 100644 --- a/layer/layer_test.go +++ b/layer/layer_test.go @@ -20,7 +20,8 @@ import ( func init() { graphdriver.ApplyUncompressedLayer = archive.UnpackLayer - vfs.CopyWithTar = archive.CopyWithTar + defaultArchiver := archive.NewDefaultArchiver() + vfs.CopyWithTar = defaultArchiver.CopyWithTar } func newVFSGraphDriver(td string) (graphdriver.Driver, error) { diff --git a/libcontainerd/client_unix.go b/libcontainerd/client_unix.go index 906026024d..456e21fceb 100644 --- a/libcontainerd/client_unix.go +++ b/libcontainerd/client_unix.go @@ -34,7 +34,7 @@ func (clnt *client) prepareBundleDir(uid, gid int) (string, error) { } if os.IsNotExist(err) || fi.Mode()&1 == 0 { p = fmt.Sprintf("%s.%d.%d", p, uid, gid) - if err := idtools.MkdirAs(p, 0700, uid, gid); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAndChown(p, 0700, idtools.IDPair{uid, gid}); err != nil && !os.IsExist(err) { return "", err } } @@ -71,7 +71,7 @@ func (clnt *client) Create(containerID string, checkpoint string, checkpointDir } }() - if err := idtools.MkdirAllAs(container.dir, 0700, uid, gid); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAllAndChown(container.dir, 0700, idtools.IDPair{uid, gid}); err != nil && !os.IsExist(err) { return err } diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 30b3c5b36f..5fb774d602 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -6,7 +6,6 @@ import ( "bytes" "compress/bzip2" "compress/gzip" - "errors" "fmt" "io" "io/ioutil" @@ -31,10 +30,6 @@ type ( Compression int // WhiteoutFormat is the format of whiteouts unpacked WhiteoutFormat int - // TarChownOptions wraps the chown options UID and GID. - TarChownOptions struct { - UID, GID int - } // TarOptions wraps the tar options. TarOptions struct { @@ -44,7 +39,7 @@ type ( NoLchown bool UIDMaps []idtools.IDMap GIDMaps []idtools.IDMap - ChownOpts *TarChownOptions + ChownOpts *idtools.IDPair IncludeSourceDir bool // WhiteoutFormat is the expected on disk format for whiteout files. // This format will be converted to the standard format on pack @@ -58,33 +53,26 @@ type ( RebaseNames map[string]string InUserNS bool } - - // Archiver allows the reuse of most utility functions of this package - // with a pluggable Untar function. Also, to facilitate the passing of - // specific id mappings for untar, an archiver can be created with maps - // which will then be passed to Untar operations - Archiver struct { - Untar func(io.Reader, string, *TarOptions) error - UIDMaps []idtools.IDMap - GIDMaps []idtools.IDMap - } - - // breakoutError is used to differentiate errors related to breaking out - // When testing archive breakout in the unit tests, this error is expected - // in order for the test to pass. - breakoutError error ) -var ( - // ErrNotImplemented is the error message of function not implemented. - ErrNotImplemented = errors.New("Function not implemented") - defaultArchiver = &Archiver{Untar: Untar, UIDMaps: nil, GIDMaps: nil} -) +// Archiver allows the reuse of most utility functions of this package +// with a pluggable Untar function. Also, to facilitate the passing of +// specific id mappings for untar, an archiver can be created with maps +// which will then be passed to Untar operations +type Archiver struct { + Untar func(io.Reader, string, *TarOptions) error + IDMappings *idtools.IDMappings +} -const ( - // HeaderSize is the size in bytes of a tar header - HeaderSize = 512 -) +// NewDefaultArchiver returns a new Archiver without any IDMappings +func NewDefaultArchiver() *Archiver { + return &Archiver{Untar: Untar, IDMappings: &idtools.IDMappings{}} +} + +// breakoutError is used to differentiate errors related to breaking out +// When testing archive breakout in the unit tests, this error is expected +// in order for the test to pass. +type breakoutError error const ( // Uncompressed represents the uncompressed. @@ -105,18 +93,6 @@ const ( OverlayWhiteoutFormat ) -// IsArchive checks for the magic bytes of a tar or any supported compression -// algorithm. -func IsArchive(header []byte) bool { - compression := DetectCompression(header) - if compression != Uncompressed { - return true - } - r := tar.NewReader(bytes.NewBuffer(header)) - _, err := r.Next() - return err == nil -} - // IsArchivePath checks if the (possibly compressed) file at the given path // starts with a tar file header. func IsArchivePath(path string) bool { @@ -369,9 +345,8 @@ type tarAppender struct { Buffer *bufio.Writer // for hardlink mapping - SeenFiles map[uint64]string - UIDMaps []idtools.IDMap - GIDMaps []idtools.IDMap + SeenFiles map[uint64]string + IDMappings *idtools.IDMappings // For packing and unpacking whiteout files in the // non standard format. The whiteout files defined @@ -380,6 +355,15 @@ type tarAppender struct { WhiteoutConverter tarWhiteoutConverter } +func newTarAppender(idMapping *idtools.IDMappings, writer io.Writer) *tarAppender { + return &tarAppender{ + SeenFiles: make(map[uint64]string), + TarWriter: tar.NewWriter(writer), + Buffer: pools.BufioWriter32KPool.Get(nil), + IDMappings: idMapping, + } +} + // canonicalTarName provides a platform-independent and consistent posix-style //path for files and directories to be archived regardless of the platform. func canonicalTarName(name string, isDir bool) (string, error) { @@ -428,21 +412,15 @@ func (ta *tarAppender) addTarFile(path, name string) error { //handle re-mapping container ID mappings back to host ID mappings before //writing tar headers/files. We skip whiteout files because they were written //by the kernel and already have proper ownership relative to the host - if !strings.HasPrefix(filepath.Base(hdr.Name), WhiteoutPrefix) && (ta.UIDMaps != nil || ta.GIDMaps != nil) { - uid, gid, err := getFileUIDGID(fi.Sys()) + if !strings.HasPrefix(filepath.Base(hdr.Name), WhiteoutPrefix) && !ta.IDMappings.Empty() { + fileIDPair, err := getFileUIDGID(fi.Sys()) if err != nil { return err } - xUID, err := idtools.ToContainer(uid, ta.UIDMaps) + hdr.Uid, hdr.Gid, err = ta.IDMappings.ToContainer(fileIDPair) if err != nil { return err } - xGID, err := idtools.ToContainer(gid, ta.GIDMaps) - if err != nil { - return err - } - hdr.Uid = xUID - hdr.Gid = xGID } if ta.WhiteoutConverter != nil { @@ -496,7 +474,7 @@ func (ta *tarAppender) addTarFile(path, name string) error { return nil } -func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, Lchown bool, chownOpts *TarChownOptions, inUserns bool) error { +func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, Lchown bool, chownOpts *idtools.IDPair, inUserns bool) error { // hdr.Mode is in linux format, which we can use for sycalls, // but for os.Foo() calls we need the mode converted to os.FileMode, // so use hdrInfo.Mode() (they differ for e.g. setuid bits) @@ -576,7 +554,7 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L // Lchown is not supported on Windows. if Lchown && runtime.GOOS != "windows" { if chownOpts == nil { - chownOpts = &TarChownOptions{UID: hdr.Uid, GID: hdr.Gid} + chownOpts = &idtools.IDPair{UID: hdr.Uid, GID: hdr.Gid} } if err := os.Lchown(path, chownOpts.UID, chownOpts.GID); err != nil { return err @@ -664,14 +642,11 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) } go func() { - ta := &tarAppender{ - TarWriter: tar.NewWriter(compressWriter), - Buffer: pools.BufioWriter32KPool.Get(nil), - SeenFiles: make(map[uint64]string), - UIDMaps: options.UIDMaps, - GIDMaps: options.GIDMaps, - WhiteoutConverter: getWhiteoutConverter(options.WhiteoutFormat), - } + ta := newTarAppender( + idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps), + compressWriter, + ) + ta.WhiteoutConverter = getWhiteoutConverter(options.WhiteoutFormat) defer func() { // Make sure to check the error on Close. @@ -827,10 +802,8 @@ func Unpack(decompressedArchive io.Reader, dest string, options *TarOptions) err defer pools.BufioReader32KPool.Put(trBuf) var dirs []*tar.Header - remappedRootUID, remappedRootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps) - if err != nil { - return err - } + idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) + rootIDs := idMappings.RootPair() whiteoutConverter := getWhiteoutConverter(options.WhiteoutFormat) // Iterate through the files in the archive. @@ -864,7 +837,7 @@ loop: parent := filepath.Dir(hdr.Name) parentPath := filepath.Join(dest, parent) if _, err := os.Lstat(parentPath); err != nil && os.IsNotExist(err) { - err = idtools.MkdirAllNewAs(parentPath, 0777, remappedRootUID, remappedRootGID) + err = idtools.MkdirAllAndChownNew(parentPath, 0777, rootIDs) if err != nil { return err } @@ -909,26 +882,8 @@ loop: } trBuf.Reset(tr) - // if the options contain a uid & gid maps, convert header uid/gid - // entries using the maps such that lchown sets the proper mapped - // uid/gid after writing the file. We only perform this mapping if - // the file isn't already owned by the remapped root UID or GID, as - // that specific uid/gid has no mapping from container -> host, and - // those files already have the proper ownership for inside the - // container. - if hdr.Uid != remappedRootUID { - xUID, err := idtools.ToHost(hdr.Uid, options.UIDMaps) - if err != nil { - return err - } - hdr.Uid = xUID - } - if hdr.Gid != remappedRootGID { - xGID, err := idtools.ToHost(hdr.Gid, options.GIDMaps) - if err != nil { - return err - } - hdr.Gid = xGID + if err := remapIDs(idMappings, hdr); err != nil { + return err } if whiteoutConverter != nil { @@ -1013,23 +968,13 @@ func (archiver *Archiver) TarUntar(src, dst string) error { return err } defer archive.Close() - - var options *TarOptions - if archiver.UIDMaps != nil || archiver.GIDMaps != nil { - options = &TarOptions{ - UIDMaps: archiver.UIDMaps, - GIDMaps: archiver.GIDMaps, - } + options := &TarOptions{ + UIDMaps: archiver.IDMappings.UIDs(), + GIDMaps: archiver.IDMappings.GIDs(), } return archiver.Untar(archive, dst, options) } -// TarUntar is a convenience function which calls Tar and Untar, with the output of one piped into the other. -// If either Tar or Untar fails, TarUntar aborts and returns the error. -func TarUntar(src, dst string) error { - return defaultArchiver.TarUntar(src, dst) -} - // UntarPath untar a file from path to a destination, src is the source tar file path. func (archiver *Archiver) UntarPath(src, dst string) error { archive, err := os.Open(src) @@ -1037,22 +982,13 @@ func (archiver *Archiver) UntarPath(src, dst string) error { return err } defer archive.Close() - var options *TarOptions - if archiver.UIDMaps != nil || archiver.GIDMaps != nil { - options = &TarOptions{ - UIDMaps: archiver.UIDMaps, - GIDMaps: archiver.GIDMaps, - } + options := &TarOptions{ + UIDMaps: archiver.IDMappings.UIDs(), + GIDMaps: archiver.IDMappings.GIDs(), } return archiver.Untar(archive, dst, options) } -// UntarPath is a convenience function which looks for an archive -// at filesystem path `src`, and unpacks it at `dst`. -func UntarPath(src, dst string) error { - return defaultArchiver.UntarPath(src, dst) -} - // CopyWithTar creates a tar archive of filesystem path `src`, and // unpacks it at filesystem path `dst`. // The archive is streamed directly with fixed buffering and no @@ -1069,27 +1005,16 @@ func (archiver *Archiver) CopyWithTar(src, dst string) error { // if this archiver is set up with ID mapping we need to create // the new destination directory with the remapped root UID/GID pair // as owner - rootUID, rootGID, err := idtools.GetRootUIDGID(archiver.UIDMaps, archiver.GIDMaps) - if err != nil { - return err - } + rootIDs := archiver.IDMappings.RootPair() // Create dst, copy src's content into it logrus.Debugf("Creating dest directory: %s", dst) - if err := idtools.MkdirAllNewAs(dst, 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(dst, 0755, rootIDs); err != nil { return err } logrus.Debugf("Calling TarUntar(%s, %s)", src, dst) return archiver.TarUntar(src, dst) } -// CopyWithTar creates a tar archive of filesystem path `src`, and -// unpacks it at filesystem path `dst`. -// The archive is streamed directly with fixed buffering and no -// intermediary disk IO. -func CopyWithTar(src, dst string) error { - return defaultArchiver.CopyWithTar(src, dst) -} - // CopyFileWithTar emulates the behavior of the 'cp' command-line // for a single file. It copies a regular file from path `src` to // path `dst`, and preserves all its metadata. @@ -1131,28 +1056,10 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) { hdr.Name = filepath.Base(dst) hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode))) - remappedRootUID, remappedRootGID, err := idtools.GetRootUIDGID(archiver.UIDMaps, archiver.GIDMaps) - if err != nil { + if err := remapIDs(archiver.IDMappings, hdr); err != nil { return err } - // only perform mapping if the file being copied isn't already owned by the - // uid or gid of the remapped root in the container - if remappedRootUID != hdr.Uid { - xUID, err := idtools.ToHost(hdr.Uid, archiver.UIDMaps) - if err != nil { - return err - } - hdr.Uid = xUID - } - if remappedRootGID != hdr.Gid { - xGID, err := idtools.ToHost(hdr.Gid, archiver.GIDMaps) - if err != nil { - return err - } - hdr.Gid = xGID - } - tw := tar.NewWriter(w) defer tw.Close() if err := tw.WriteHeader(hdr); err != nil { @@ -1176,16 +1083,10 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) { return err } -// CopyFileWithTar emulates the behavior of the 'cp' command-line -// for a single file. It copies a regular file from path `src` to -// path `dst`, and preserves all its metadata. -// -// Destination handling is in an operating specific manner depending -// where the daemon is running. If `dst` ends with a trailing slash -// the final destination path will be `dst/base(src)` (Linux) or -// `dst\base(src)` (Windows). -func CopyFileWithTar(src, dst string) (err error) { - return defaultArchiver.CopyFileWithTar(src, dst) +func remapIDs(idMappings *idtools.IDMappings, hdr *tar.Header) error { + ids, err := idMappings.ToHost(idtools.IDPair{UID: hdr.Uid, GID: hdr.Gid}) + hdr.Uid, hdr.Gid = ids.UID, ids.GID + return err } // cmdStream executes a command, and returns its stdout as a stream. diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index e351a0455b..e85878fd0f 100644 --- a/pkg/archive/archive_test.go +++ b/pkg/archive/archive_test.go @@ -27,35 +27,22 @@ func init() { } } -func TestIsArchiveNilHeader(t *testing.T) { - out := IsArchive(nil) - if out { - t.Fatalf("isArchive should return false as nil is not a valid archive header") - } +var defaultArchiver = NewDefaultArchiver() + +func defaultTarUntar(src, dst string) error { + return defaultArchiver.TarUntar(src, dst) } -func TestIsArchiveInvalidHeader(t *testing.T) { - header := []byte{0x00, 0x01, 0x02} - out := IsArchive(header) - if out { - t.Fatalf("isArchive should return false as %s is not a valid archive header", header) - } +func defaultUntarPath(src, dst string) error { + return defaultArchiver.UntarPath(src, dst) } -func TestIsArchiveBzip2(t *testing.T) { - header := []byte{0x42, 0x5A, 0x68} - out := IsArchive(header) - if !out { - t.Fatalf("isArchive should return true as %s is a bz2 header", header) - } +func defaultCopyFileWithTar(src, dst string) (err error) { + return defaultArchiver.CopyFileWithTar(src, dst) } -func TestIsArchive7zip(t *testing.T) { - header := []byte{0x50, 0x4b, 0x03, 0x04} - out := IsArchive(header) - if out { - t.Fatalf("isArchive should return false as %s is a 7z header and it is not supported", header) - } +func defaultCopyWithTar(src, dst string) error { + return defaultArchiver.CopyWithTar(src, dst) } func TestIsArchivePathDir(t *testing.T) { @@ -301,7 +288,7 @@ func TestUntarPathWithInvalidDest(t *testing.T) { t.Fatal(err) } - err = UntarPath(tarFile, invalidDestFolder) + err = defaultUntarPath(tarFile, invalidDestFolder) if err == nil { t.Fatalf("UntarPath with invalid destination path should throw an error.") } @@ -313,7 +300,7 @@ func TestUntarPathWithInvalidSrc(t *testing.T) { t.Fatalf("Fail to create the destination file") } defer os.RemoveAll(dest) - err = UntarPath("/invalid/path", dest) + err = defaultUntarPath("/invalid/path", dest) if err == nil { t.Fatalf("UntarPath with invalid src path should throw an error.") } @@ -348,7 +335,7 @@ func TestUntarPath(t *testing.T) { t.Fatal(err) } - err = UntarPath(tarFile, destFolder) + err = defaultUntarPath(tarFile, destFolder) if err != nil { t.Fatalf("UntarPath shouldn't throw an error, %s.", err) } @@ -387,7 +374,7 @@ func TestUntarPathWithDestinationFile(t *testing.T) { if err != nil { t.Fatalf("Fail to create the destination file") } - err = UntarPath(tarFile, destFile) + err = defaultUntarPath(tarFile, destFile) if err == nil { t.Fatalf("UntarPath should throw an error if the destination if a file") } @@ -430,7 +417,7 @@ func TestUntarPathWithDestinationSrcFileAsFolder(t *testing.T) { if err != nil { t.Fatal(err) } - err = UntarPath(tarFile, destFolder) + err = defaultUntarPath(tarFile, destFolder) if err != nil { t.Fatalf("UntarPath should throw not throw an error if the extracted file already exists and is a folder") } @@ -447,7 +434,7 @@ func TestCopyWithTarInvalidSrc(t *testing.T) { if err != nil { t.Fatal(err) } - err = CopyWithTar(invalidSrc, destFolder) + err = defaultCopyWithTar(invalidSrc, destFolder) if err == nil { t.Fatalf("archiver.CopyWithTar with invalid src path should throw an error.") } @@ -464,7 +451,7 @@ func TestCopyWithTarInexistentDestWillCreateIt(t *testing.T) { if err != nil { t.Fatal(err) } - err = CopyWithTar(srcFolder, inexistentDestFolder) + err = defaultCopyWithTar(srcFolder, inexistentDestFolder) if err != nil { t.Fatalf("CopyWithTar with an inexistent folder shouldn't fail.") } @@ -493,7 +480,7 @@ func TestCopyWithTarSrcFile(t *testing.T) { t.Fatal(err) } ioutil.WriteFile(src, []byte("content"), 0777) - err = CopyWithTar(src, dest) + err = defaultCopyWithTar(src, dest) if err != nil { t.Fatalf("archiver.CopyWithTar shouldn't throw an error, %s.", err) } @@ -522,7 +509,7 @@ func TestCopyWithTarSrcFolder(t *testing.T) { t.Fatal(err) } ioutil.WriteFile(filepath.Join(src, "file"), []byte("content"), 0777) - err = CopyWithTar(src, dest) + err = defaultCopyWithTar(src, dest) if err != nil { t.Fatalf("archiver.CopyWithTar shouldn't throw an error, %s.", err) } @@ -545,7 +532,7 @@ func TestCopyFileWithTarInvalidSrc(t *testing.T) { t.Fatal(err) } invalidFile := filepath.Join(tempFolder, "doesnotexists") - err = CopyFileWithTar(invalidFile, destFolder) + err = defaultCopyFileWithTar(invalidFile, destFolder) if err == nil { t.Fatalf("archiver.CopyWithTar with invalid src path should throw an error.") } @@ -563,7 +550,7 @@ func TestCopyFileWithTarInexistentDestWillCreateIt(t *testing.T) { if err != nil { t.Fatal(err) } - err = CopyFileWithTar(srcFile, inexistentDestFolder) + err = defaultCopyFileWithTar(srcFile, inexistentDestFolder) if err != nil { t.Fatalf("CopyWithTar with an inexistent folder shouldn't fail.") } @@ -590,7 +577,7 @@ func TestCopyFileWithTarSrcFolder(t *testing.T) { if err != nil { t.Fatal(err) } - err = CopyFileWithTar(src, dest) + err = defaultCopyFileWithTar(src, dest) if err == nil { t.Fatalf("CopyFileWithTar should throw an error with a folder.") } @@ -614,7 +601,7 @@ func TestCopyFileWithTarSrcFile(t *testing.T) { t.Fatal(err) } ioutil.WriteFile(src, []byte("content"), 0777) - err = CopyWithTar(src, dest+"/") + err = defaultCopyWithTar(src, dest+"/") if err != nil { t.Fatalf("archiver.CopyFileWithTar shouldn't throw an error, %s.", err) } @@ -657,7 +644,7 @@ func checkNoChanges(fileNum int, hardlinks bool) error { return err } - err = TarUntar(srcDir, destDir) + err = defaultTarUntar(srcDir, destDir) if err != nil { return err } @@ -871,7 +858,7 @@ func BenchmarkTarUntar(b *testing.B) { b.ResetTimer() b.SetBytes(int64(n)) for n := 0; n < b.N; n++ { - err := TarUntar(origin, target) + err := defaultTarUntar(origin, target) if err != nil { b.Fatal(err) } @@ -899,7 +886,7 @@ func BenchmarkTarUntarWithLinks(b *testing.B) { b.ResetTimer() b.SetBytes(int64(n)) for n := 0; n < b.N; n++ { - err := TarUntar(origin, target) + err := defaultTarUntar(origin, target) if err != nil { b.Fatal(err) } diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go index 68d3c97d27..33ee88766f 100644 --- a/pkg/archive/archive_unix.go +++ b/pkg/archive/archive_unix.go @@ -9,6 +9,7 @@ import ( "path/filepath" "syscall" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/system" rsystem "github.com/opencontainers/runc/libcontainer/system" ) @@ -72,13 +73,13 @@ func getInodeFromStat(stat interface{}) (inode uint64, err error) { return } -func getFileUIDGID(stat interface{}) (int, int, error) { +func getFileUIDGID(stat interface{}) (idtools.IDPair, error) { s, ok := stat.(*syscall.Stat_t) if !ok { - return -1, -1, errors.New("cannot convert stat value to syscall.Stat_t") + return idtools.IDPair{}, errors.New("cannot convert stat value to syscall.Stat_t") } - return int(s.Uid), int(s.Gid), nil + return idtools.IDPair{UID: int(s.Uid), GID: int(s.Gid)}, nil } func major(device uint64) uint64 { diff --git a/pkg/archive/archive_windows.go b/pkg/archive/archive_windows.go index 9c7094738d..a22410c039 100644 --- a/pkg/archive/archive_windows.go +++ b/pkg/archive/archive_windows.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/longpath" ) @@ -72,7 +73,7 @@ func handleLChmod(hdr *tar.Header, path string, hdrInfo os.FileInfo) error { return nil } -func getFileUIDGID(stat interface{}) (int, int, error) { +func getFileUIDGID(stat interface{}) (idtools.IDPair, error) { // no notion of file ownership mapping yet on Windows - return 0, 0, nil + return idtools.IDPair{0, 0}, nil } diff --git a/pkg/archive/archive_windows_test.go b/pkg/archive/archive_windows_test.go index 9dd937f296..685e114baf 100644 --- a/pkg/archive/archive_windows_test.go +++ b/pkg/archive/archive_windows_test.go @@ -27,7 +27,7 @@ func TestCopyFileWithInvalidDest(t *testing.T) { t.Fatal(err) } ioutil.WriteFile(src, []byte("content"), 0777) - err = CopyWithTar(src, dest) + err = defaultCopyWithTar(src, dest) if err == nil { t.Fatalf("archiver.CopyWithTar should throw an error on invalid dest.") } diff --git a/pkg/archive/changes.go b/pkg/archive/changes.go index ca2c0ca1bf..5ca39b7215 100644 --- a/pkg/archive/changes.go +++ b/pkg/archive/changes.go @@ -394,13 +394,8 @@ func ChangesSize(newDir string, changes []Change) int64 { func ExportChanges(dir string, changes []Change, uidMaps, gidMaps []idtools.IDMap) (io.ReadCloser, error) { reader, writer := io.Pipe() go func() { - ta := &tarAppender{ - TarWriter: tar.NewWriter(writer), - Buffer: pools.BufioWriter32KPool.Get(nil), - SeenFiles: make(map[uint64]string), - UIDMaps: uidMaps, - GIDMaps: gidMaps, - } + ta := newTarAppender(idtools.NewIDMappingsFromMaps(uidMaps, gidMaps), writer) + // this buffer is needed for the duration of this piped stream defer pools.BufioWriter32KPool.Put(ta.Buffer) diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go index 2292193942..d20854e2a2 100644 --- a/pkg/archive/diff.go +++ b/pkg/archive/diff.go @@ -33,10 +33,7 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, if options.ExcludePatterns == nil { options.ExcludePatterns = []string{} } - remappedRootUID, remappedRootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps) - if err != nil { - return 0, err - } + idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) aufsTempdir := "" aufsHardlinks := make(map[string]*tar.Header) @@ -195,27 +192,10 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, srcData = tmpFile } - // if the options contain a uid & gid maps, convert header uid/gid - // entries using the maps such that lchown sets the proper mapped - // uid/gid after writing the file. We only perform this mapping if - // the file isn't already owned by the remapped root UID or GID, as - // that specific uid/gid has no mapping from container -> host, and - // those files already have the proper ownership for inside the - // container. - if srcHdr.Uid != remappedRootUID { - xUID, err := idtools.ToHost(srcHdr.Uid, options.UIDMaps) - if err != nil { - return 0, err - } - srcHdr.Uid = xUID - } - if srcHdr.Gid != remappedRootGID { - xGID, err := idtools.ToHost(srcHdr.Gid, options.GIDMaps) - if err != nil { - return 0, err - } - srcHdr.Gid = xGID + if err := remapIDs(idMappings, srcHdr); err != nil { + return 0, err } + if err := createTarFile(path, dest, srcHdr, srcData, true, nil, options.InUserNS); err != nil { return 0, err } diff --git a/pkg/chrootarchive/archive.go b/pkg/chrootarchive/archive.go index a7814f5b90..7604418767 100644 --- a/pkg/chrootarchive/archive.go +++ b/pkg/chrootarchive/archive.go @@ -11,7 +11,13 @@ import ( "github.com/docker/docker/pkg/idtools" ) -var chrootArchiver = &archive.Archiver{Untar: Untar} +// NewArchiver returns a new Archiver which uses chrootarchive.Untar +func NewArchiver(idMappings *idtools.IDMappings) *archive.Archiver { + if idMappings == nil { + idMappings = &idtools.IDMappings{} + } + return &archive.Archiver{Untar: Untar, IDMappings: idMappings} +} // Untar reads a stream of bytes from `archive`, parses it as a tar archive, // and unpacks it into the directory at `dest`. @@ -30,7 +36,6 @@ func UntarUncompressed(tarArchive io.Reader, dest string, options *archive.TarOp // Handler for teasing out the automatic decompression func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions, decompress bool) error { - if tarArchive == nil { return fmt.Errorf("Empty archive") } @@ -41,14 +46,12 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions options.ExcludePatterns = []string{} } - rootUID, rootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps) - if err != nil { - return err - } + idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) + rootIDs := idMappings.RootPair() dest = filepath.Clean(dest) if _, err := os.Stat(dest); os.IsNotExist(err) { - if err := idtools.MkdirAllNewAs(dest, 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(dest, 0755, rootIDs); err != nil { return err } } @@ -65,33 +68,3 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions return invokeUnpack(r, dest, options) } - -// TarUntar is a convenience function which calls Tar and Untar, with the output of one piped into the other. -// If either Tar or Untar fails, TarUntar aborts and returns the error. -func TarUntar(src, dst string) error { - return chrootArchiver.TarUntar(src, dst) -} - -// CopyWithTar creates a tar archive of filesystem path `src`, and -// unpacks it at filesystem path `dst`. -// The archive is streamed directly with fixed buffering and no -// intermediary disk IO. -func CopyWithTar(src, dst string) error { - return chrootArchiver.CopyWithTar(src, dst) -} - -// CopyFileWithTar emulates the behavior of the 'cp' command-line -// for a single file. It copies a regular file from path `src` to -// path `dst`, and preserves all its metadata. -// -// If `dst` ends with a trailing slash '/' ('\' on Windows), the final -// destination path will be `dst/base(src)` or `dst\base(src)` -func CopyFileWithTar(src, dst string) (err error) { - return chrootArchiver.CopyFileWithTar(src, dst) -} - -// UntarPath is a convenience function which looks for an archive -// at filesystem path `src`, and unpacks it at `dst`. -func UntarPath(src, dst string) error { - return chrootArchiver.UntarPath(src, dst) -} diff --git a/pkg/chrootarchive/archive_test.go b/pkg/chrootarchive/archive_test.go index 80e54a0edc..4780f3be08 100644 --- a/pkg/chrootarchive/archive_test.go +++ b/pkg/chrootarchive/archive_test.go @@ -22,6 +22,24 @@ func init() { reexec.Init() } +var chrootArchiver = NewArchiver(nil) + +func TarUntar(src, dst string) error { + return chrootArchiver.TarUntar(src, dst) +} + +func CopyFileWithTar(src, dst string) (err error) { + return chrootArchiver.CopyFileWithTar(src, dst) +} + +func UntarPath(src, dst string) error { + return chrootArchiver.UntarPath(src, dst) +} + +func CopyWithTar(src, dst string) error { + return chrootArchiver.CopyWithTar(src, dst) +} + func TestChrootTarUntar(t *testing.T) { tmpdir, err := ioutil.TempDir("", "docker-TestChrootTarUntar") if err != nil { diff --git a/pkg/idtools/idtools.go b/pkg/idtools/idtools.go index 6bca466286..68a072db22 100644 --- a/pkg/idtools/idtools.go +++ b/pkg/idtools/idtools.go @@ -37,49 +37,56 @@ const ( // MkdirAllAs creates a directory (include any along the path) and then modifies // ownership to the requested uid/gid. If the directory already exists, this // function will still change ownership to the requested uid/gid pair. +// Deprecated: Use MkdirAllAndChown func MkdirAllAs(path string, mode os.FileMode, ownerUID, ownerGID int) error { return mkdirAs(path, mode, ownerUID, ownerGID, true, true) } -// MkdirAllNewAs creates a directory (include any along the path) and then modifies -// ownership ONLY of newly created directories to the requested uid/gid. If the -// directories along the path exist, no change of ownership will be performed -func MkdirAllNewAs(path string, mode os.FileMode, ownerUID, ownerGID int) error { - return mkdirAs(path, mode, ownerUID, ownerGID, true, false) -} - // MkdirAs creates a directory and then modifies ownership to the requested uid/gid. // If the directory already exists, this function still changes ownership +// Deprecated: Use MkdirAndChown with a IDPair func MkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int) error { return mkdirAs(path, mode, ownerUID, ownerGID, false, true) } +// MkdirAllAndChown creates a directory (include any along the path) and then modifies +// ownership to the requested uid/gid. If the directory already exists, this +// function will still change ownership to the requested uid/gid pair. +func MkdirAllAndChown(path string, mode os.FileMode, ids IDPair) error { + return mkdirAs(path, mode, ids.UID, ids.GID, true, true) +} + +// MkdirAndChown creates a directory and then modifies ownership to the requested uid/gid. +// If the directory already exists, this function still changes ownership +func MkdirAndChown(path string, mode os.FileMode, ids IDPair) error { + return mkdirAs(path, mode, ids.UID, ids.GID, false, true) +} + +// MkdirAllAndChownNew creates a directory (include any along the path) and then modifies +// ownership ONLY of newly created directories to the requested uid/gid. If the +// directories along the path exist, no change of ownership will be performed +func MkdirAllAndChownNew(path string, mode os.FileMode, ids IDPair) error { + return mkdirAs(path, mode, ids.UID, ids.GID, true, false) +} + // GetRootUIDGID retrieves the remapped root uid/gid pair from the set of maps. // If the maps are empty, then the root uid/gid will default to "real" 0/0 func GetRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) { - var uid, gid int - - if uidMap != nil { - xUID, err := ToHost(0, uidMap) - if err != nil { - return -1, -1, err - } - uid = xUID + uid, err := toHost(0, uidMap) + if err != nil { + return -1, -1, err } - if gidMap != nil { - xGID, err := ToHost(0, gidMap) - if err != nil { - return -1, -1, err - } - gid = xGID + gid, err := toHost(0, gidMap) + if err != nil { + return -1, -1, err } return uid, gid, nil } -// ToContainer takes an id mapping, and uses it to translate a +// toContainer takes an id mapping, and uses it to translate a // host ID to the remapped ID. If no map is provided, then the translation // assumes a 1-to-1 mapping and returns the passed in id -func ToContainer(hostID int, idMap []IDMap) (int, error) { +func toContainer(hostID int, idMap []IDMap) (int, error) { if idMap == nil { return hostID, nil } @@ -92,10 +99,10 @@ func ToContainer(hostID int, idMap []IDMap) (int, error) { return -1, fmt.Errorf("Host ID %d cannot be mapped to a container ID", hostID) } -// ToHost takes an id mapping and a remapped ID, and translates the +// toHost takes an id mapping and a remapped ID, and translates the // ID to the mapped host ID. If no map is provided, then the translation // assumes a 1-to-1 mapping and returns the passed in id # -func ToHost(contID int, idMap []IDMap) (int, error) { +func toHost(contID int, idMap []IDMap) (int, error) { if idMap == nil { return contID, nil } @@ -108,26 +115,101 @@ func ToHost(contID int, idMap []IDMap) (int, error) { return -1, fmt.Errorf("Container ID %d cannot be mapped to a host ID", contID) } -// CreateIDMappings takes a requested user and group name and +// IDPair is a UID and GID pair +type IDPair struct { + UID int + GID int +} + +// IDMappings contains a mappings of UIDs and GIDs +type IDMappings struct { + uids []IDMap + gids []IDMap +} + +// NewIDMappings takes a requested user and group name and // using the data from /etc/sub{uid,gid} ranges, creates the // proper uid and gid remapping ranges for that user/group pair -func CreateIDMappings(username, groupname string) ([]IDMap, []IDMap, error) { +func NewIDMappings(username, groupname string) (*IDMappings, error) { subuidRanges, err := parseSubuid(username) if err != nil { - return nil, nil, err + return nil, err } subgidRanges, err := parseSubgid(groupname) if err != nil { - return nil, nil, err + return nil, err } if len(subuidRanges) == 0 { - return nil, nil, fmt.Errorf("No subuid ranges found for user %q", username) + return nil, fmt.Errorf("No subuid ranges found for user %q", username) } if len(subgidRanges) == 0 { - return nil, nil, fmt.Errorf("No subgid ranges found for group %q", groupname) + return nil, fmt.Errorf("No subgid ranges found for group %q", groupname) } - return createIDMap(subuidRanges), createIDMap(subgidRanges), nil + return &IDMappings{ + uids: createIDMap(subuidRanges), + gids: createIDMap(subgidRanges), + }, nil +} + +// NewIDMappingsFromMaps creates a new mapping from two slices +// Deprecated: this is a temporary shim while transitioning to IDMapping +func NewIDMappingsFromMaps(uids []IDMap, gids []IDMap) *IDMappings { + return &IDMappings{uids: uids, gids: gids} +} + +// RootPair returns a uid and gid pair for the root user. The error is ignored +// because a root user always exists, and the defaults are correct when the uid +// and gid maps are empty. +func (i *IDMappings) RootPair() IDPair { + uid, gid, _ := GetRootUIDGID(i.uids, i.gids) + return IDPair{UID: uid, GID: gid} +} + +// ToHost returns the host UID and GID for the container uid, gid. +// Remapping is only performed if the ids aren't already the remapped root ids +func (i *IDMappings) ToHost(pair IDPair) (IDPair, error) { + var err error + target := i.RootPair() + + if pair.UID != target.UID { + target.UID, err = toHost(pair.UID, i.uids) + if err != nil { + return target, err + } + } + + if pair.GID != target.GID { + target.GID, err = toHost(pair.GID, i.gids) + } + return target, err +} + +// ToContainer returns the container UID and GID for the host uid and gid +func (i *IDMappings) ToContainer(pair IDPair) (int, int, error) { + uid, err := toContainer(pair.UID, i.uids) + if err != nil { + return -1, -1, err + } + gid, err := toContainer(pair.GID, i.gids) + return uid, gid, err +} + +// Empty returns true if there are no id mappings +func (i *IDMappings) Empty() bool { + return len(i.uids) == 0 && len(i.gids) == 0 +} + +// UIDs return the UID mapping +// TODO: remove this once everything has been refactored to use pairs +func (i *IDMappings) UIDs() []IDMap { + return i.uids +} + +// GIDs return the UID mapping +// TODO: remove this once everything has been refactored to use pairs +func (i *IDMappings) GIDs() []IDMap { + return i.gids } func createIDMap(subidRanges ranges) []IDMap { diff --git a/pkg/idtools/idtools_unix.go b/pkg/idtools/idtools_unix.go index 7c7e82aee2..0b28249fa8 100644 --- a/pkg/idtools/idtools_unix.go +++ b/pkg/idtools/idtools_unix.go @@ -69,15 +69,15 @@ func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chown // CanAccess takes a valid (existing) directory and a uid, gid pair and determines // if that uid, gid pair has access (execute bit) to the directory -func CanAccess(path string, uid, gid int) bool { +func CanAccess(path string, pair IDPair) bool { statInfo, err := system.Stat(path) if err != nil { return false } fileMode := os.FileMode(statInfo.Mode()) permBits := fileMode.Perm() - return accessible(statInfo.UID() == uint32(uid), - statInfo.GID() == uint32(gid), permBits) + return accessible(statInfo.UID() == uint32(pair.UID), + statInfo.GID() == uint32(pair.GID), permBits) } func accessible(isOwner, isGroup bool, perms os.FileMode) bool { diff --git a/pkg/idtools/idtools_unix_test.go b/pkg/idtools/idtools_unix_test.go index 540d3079ee..31522a547d 100644 --- a/pkg/idtools/idtools_unix_test.go +++ b/pkg/idtools/idtools_unix_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "syscall" "testing" + + "github.com/stretchr/testify/require" ) type node struct { @@ -76,12 +78,9 @@ func TestMkdirAllAs(t *testing.T) { } } -func TestMkdirAllNewAs(t *testing.T) { - +func TestMkdirAllAndChownNew(t *testing.T) { dirName, err := ioutil.TempDir("", "mkdirnew") - if err != nil { - t.Fatalf("Couldn't create temp dir: %v", err) - } + require.NoError(t, err) defer os.RemoveAll(dirName) testTree := map[string]node{ @@ -91,49 +90,32 @@ func TestMkdirAllNewAs(t *testing.T) { "lib/x86_64": {45, 45}, "lib/x86_64/share": {1, 1}, } - - if err := buildTree(dirName, testTree); err != nil { - t.Fatal(err) - } + require.NoError(t, buildTree(dirName, testTree)) // test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid - if err := MkdirAllNewAs(filepath.Join(dirName, "usr", "share"), 0755, 99, 99); err != nil { - t.Fatal(err) - } + err = MkdirAllAndChownNew(filepath.Join(dirName, "usr", "share"), 0755, IDPair{99, 99}) + require.NoError(t, err) + testTree["usr/share"] = node{99, 99} verifyTree, err := readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, compareTrees(testTree, verifyTree)) // test 2-deep new directories--both should be owned by the uid/gid pair - if err := MkdirAllNewAs(filepath.Join(dirName, "lib", "some", "other"), 0755, 101, 101); err != nil { - t.Fatal(err) - } + err = MkdirAllAndChownNew(filepath.Join(dirName, "lib", "some", "other"), 0755, IDPair{101, 101}) + require.NoError(t, err) testTree["lib/some"] = node{101, 101} testTree["lib/some/other"] = node{101, 101} verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, compareTrees(testTree, verifyTree)) // test a directory that already exists; should NOT be chowned - if err := MkdirAllNewAs(filepath.Join(dirName, "usr"), 0755, 102, 102); err != nil { - t.Fatal(err) - } + err = MkdirAllAndChownNew(filepath.Join(dirName, "usr"), 0755, IDPair{102, 102}) + require.NoError(t, err) verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, compareTrees(testTree, verifyTree)) } func TestMkdirAs(t *testing.T) { diff --git a/pkg/idtools/idtools_windows.go b/pkg/idtools/idtools_windows.go index 49f67e78c1..8ed8353060 100644 --- a/pkg/idtools/idtools_windows.go +++ b/pkg/idtools/idtools_windows.go @@ -20,6 +20,6 @@ func mkdirAs(path string, mode os.FileMode, ownerUID, ownerGID int, mkAll, chown // CanAccess takes a valid (existing) directory and a uid, gid pair and determines // if that uid, gid pair has access (execute bit) to the directory // Windows does not require/support this function, so always return true -func CanAccess(path string, uid, gid int) bool { +func CanAccess(path string, pair IDPair) bool { return true } diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index 80fc041623..5396b15bec 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -15,6 +15,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/daemon/initlayer" "github.com/docker/docker/libcontainerd" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/plugins" "github.com/docker/docker/pkg/stringid" @@ -58,7 +59,7 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { } } - if err := initlayer.Setup(filepath.Join(pm.config.Root, p.PluginObj.ID, rootFSFileName), 0, 0); err != nil { + if err := initlayer.Setup(filepath.Join(pm.config.Root, p.PluginObj.ID, rootFSFileName), idtools.IDPair{0, 0}); err != nil { return errors.WithStack(err) } diff --git a/volume/local/local.go b/volume/local/local.go index 6631423bbc..43ba1e1db7 100644 --- a/volume/local/local.go +++ b/volume/local/local.go @@ -55,10 +55,10 @@ type activeMount struct { // New instantiates a new Root instance with the provided scope. Scope // is the base path that the Root instance uses to store its // volumes. The base path is created here if it does not exist. -func New(scope string, rootUID, rootGID int) (*Root, error) { +func New(scope string, rootIDs idtools.IDPair) (*Root, error) { rootDirectory := filepath.Join(scope, volumesPathName) - if err := idtools.MkdirAllAs(rootDirectory, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(rootDirectory, 0700, rootIDs); err != nil { return nil, err } @@ -66,8 +66,7 @@ func New(scope string, rootUID, rootGID int) (*Root, error) { scope: scope, path: rootDirectory, volumes: make(map[string]*localVolume), - rootUID: rootUID, - rootGID: rootGID, + rootIDs: rootIDs, } dirs, err := ioutil.ReadDir(rootDirectory) @@ -125,8 +124,7 @@ type Root struct { scope string path string volumes map[string]*localVolume - rootUID int - rootGID int + rootIDs idtools.IDPair } // List lists all the volumes @@ -167,7 +165,7 @@ func (r *Root) Create(name string, opts map[string]string) (volume.Volume, error } path := r.DataPath(name) - if err := idtools.MkdirAllAs(path, 0755, r.rootUID, r.rootGID); err != nil { + if err := idtools.MkdirAllAndChown(path, 0755, r.rootIDs); err != nil { if os.IsExist(err) { return nil, fmt.Errorf("volume already exists under %s", filepath.Dir(path)) } diff --git a/volume/local/local_test.go b/volume/local/local_test.go index f5a519b883..2353391aa5 100644 --- a/volume/local/local_test.go +++ b/volume/local/local_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/mount" ) @@ -40,7 +41,7 @@ func TestRemove(t *testing.T) { } defer os.RemoveAll(rootDir) - r, err := New(rootDir, 0, 0) + r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -82,7 +83,7 @@ func TestInitializeWithVolumes(t *testing.T) { } defer os.RemoveAll(rootDir) - r, err := New(rootDir, 0, 0) + r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -92,7 +93,7 @@ func TestInitializeWithVolumes(t *testing.T) { t.Fatal(err) } - r, err = New(rootDir, 0, 0) + r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -114,7 +115,7 @@ func TestCreate(t *testing.T) { } defer os.RemoveAll(rootDir) - r, err := New(rootDir, 0, 0) + r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -151,7 +152,7 @@ func TestCreate(t *testing.T) { } } - r, err = New(rootDir, 0, 0) + r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -189,7 +190,7 @@ func TestCreateWithOpts(t *testing.T) { } defer os.RemoveAll(rootDir) - r, err := New(rootDir, 0, 0) + r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -270,7 +271,7 @@ func TestCreateWithOpts(t *testing.T) { t.Fatal("expected mount to still be active") } - r, err = New(rootDir, 0, 0) + r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -292,7 +293,7 @@ func TestRealodNoOpts(t *testing.T) { } defer os.RemoveAll(rootDir) - r, err := New(rootDir, 0, 0) + r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } @@ -320,7 +321,7 @@ func TestRealodNoOpts(t *testing.T) { t.Fatal(err) } - r, err = New(rootDir, 0, 0) + r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) if err != nil { t.Fatal(err) } diff --git a/volume/volume.go b/volume/volume.go index 4d761fa10c..2abfcc7b58 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -151,7 +151,7 @@ func (m *MountPoint) Cleanup() error { // configured, or creating the source directory if supplied. // The, optional, checkFun parameter allows doing additional checking // before creating the source directory on the host. -func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int, checkFun func(m *MountPoint) error) (path string, err error) { +func (m *MountPoint) Setup(mountLabel string, rootIDs idtools.IDPair, checkFun func(m *MountPoint) error) (path string, err error) { defer func() { if err == nil { if label.RelabelNeeded(m.Mode) { @@ -196,7 +196,7 @@ func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int, checkFun fun } // idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory) // also, makes sure that if the directory is created, the correct remapped rootUID/rootGID will own it - if err := idtools.MkdirAllNewAs(m.Source, 0755, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChownNew(m.Source, 0755, rootIDs); err != nil { if perr, ok := err.(*os.PathError); ok { if perr.Err != syscall.ENOTDIR { return "", errors.Wrapf(err, "error while creating mount source path '%s'", m.Source)