From 779caabedf9cd4762a1be3c4f1cc3f84008f91aa Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 19 May 2017 18:06:46 -0400 Subject: [PATCH 1/7] Partial refactor of UID/GID usage to use a unified struct. Signed-off-by: Daniel Nephin Upstream-commit: 09cd96c5ad2de369912cdf708c3c50f41e4586ac Component: engine --- components/engine/container/container.go | 4 +- components/engine/daemon/archive.go | 13 ++-- .../engine/daemon/archive_tarcopyoptions.go | 5 +- .../daemon/container_operations_unix.go | 24 +++---- components/engine/daemon/create.go | 6 +- components/engine/daemon/create_unix.go | 4 +- components/engine/daemon/daemon.go | 52 +++++--------- components/engine/daemon/daemon_test.go | 3 +- components/engine/daemon/daemon_unix.go | 26 ++++--- components/engine/daemon/daemon_unix_test.go | 3 +- components/engine/daemon/daemon_windows.go | 6 +- components/engine/daemon/export.go | 5 +- components/engine/daemon/info.go | 4 +- .../engine/daemon/initlayer/setup_unix.go | 8 +-- .../engine/daemon/initlayer/setup_windows.go | 6 +- components/engine/daemon/oci_linux.go | 10 +-- components/engine/daemon/oci_solaris.go | 4 +- components/engine/daemon/volumes_unix.go | 8 +-- components/engine/daemon/volumes_windows.go | 3 +- components/engine/daemon/workdir.go | 4 +- components/engine/layer/layer_store.go | 7 +- components/engine/pkg/idtools/idtools.go | 70 +++++++++++++++++-- components/engine/pkg/idtools/idtools_unix.go | 6 +- .../engine/pkg/idtools/idtools_windows.go | 2 +- components/engine/plugin/manager_linux.go | 3 +- components/engine/volume/local/local.go | 12 ++-- components/engine/volume/local/local_test.go | 19 ++--- components/engine/volume/volume.go | 4 +- 28 files changed, 180 insertions(+), 141 deletions(-) diff --git a/components/engine/container/container.go b/components/engine/container/container.go index e89340610c..f22b4aa574 100644 --- a/components/engine/container/container.go +++ b/components/engine/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/components/engine/daemon/archive.go b/components/engine/daemon/archive.go index dbef0b4d48..c41298df0e 100644 --- a/components/engine/daemon/archive.go +++ b/components/engine/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,11 +413,10 @@ 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, + UIDMaps: daemon.idMappings.UIDs(), + GIDMaps: daemon.idMappings.GIDs(), } src, err := os.Stat(fullSrcPath) @@ -430,7 +429,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 +458,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/components/engine/daemon/archive_tarcopyoptions.go b/components/engine/daemon/archive_tarcopyoptions.go index cbd1fc2cee..fe7722fdb4 100644 --- a/components/engine/daemon/archive_tarcopyoptions.go +++ b/components/engine/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/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 6889c10956..5ae7a68f0b 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/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/components/engine/daemon/create.go b/components/engine/daemon/create.go index de129ca60d..52b4b0e6fa 100644 --- a/components/engine/daemon/create.go +++ b/components/engine/daemon/create.go @@ -117,14 +117,14 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( return nil, err } - rootUID, rootGID, err := idtools.GetRootUIDGID(daemon.uidMaps, daemon.gidMaps) + rootIDs, err := daemon.idMappings.RootPair() if err != nil { return nil, err } - if err := idtools.MkdirAs(container.Root, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAndChown(container.Root, 0700, rootIDs); 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/components/engine/daemon/create_unix.go b/components/engine/daemon/create_unix.go index 7b067bdc22..27471e7556 100644 --- a/components/engine/daemon/create_unix.go +++ b/components/engine/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/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index 05017775d4..3e8b3106a2 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/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,11 +523,11 @@ 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) + rootIDs, err := idMappings.RootPair() if err != nil { return nil, err } @@ -538,7 +537,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe } // 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 +586,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 +631,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 +663,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 +726,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 +967,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 +990,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 +1006,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,16 +1153,16 @@ 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) + rootIDs, err := idMappings.RootPair() if err != nil { return err } - if err := setupDaemonRoot(config, realRoot, rootUID, rootGID); err != nil { + if err := setupDaemonRoot(config, realRoot, rootIDs); err != nil { return err } diff --git a/components/engine/daemon/daemon_test.go b/components/engine/daemon/daemon_test.go index eaa3be44d8..e728abddf8 100644 --- a/components/engine/daemon/daemon_test.go +++ b/components/engine/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/components/engine/daemon/daemon_unix.go b/components/engine/daemon/daemon_unix.go index 031cdc4a8b..07c323d63f 100644 --- a/components/engine/daemon/daemon_unix.go +++ b/components/engine/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/components/engine/daemon/daemon_unix_test.go b/components/engine/daemon/daemon_unix_test.go index e8afe629e0..a2847bfbee 100644 --- a/components/engine/daemon/daemon_unix_test.go +++ b/components/engine/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/components/engine/daemon/daemon_windows.go b/components/engine/daemon/daemon_windows.go index efe1b1f6a0..aced740df3 100644 --- a/components/engine/daemon/daemon_windows.go +++ b/components/engine/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/components/engine/daemon/export.go b/components/engine/daemon/export.go index 5ef6dbb0e5..402e67583d 100644 --- a/components/engine/daemon/export.go +++ b/components/engine/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/components/engine/daemon/info.go b/components/engine/daemon/info.go index 2be4b395fa..3cadea791e 100644 --- a/components/engine/daemon/info.go +++ b/components/engine/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/components/engine/daemon/initlayer/setup_unix.go b/components/engine/daemon/initlayer/setup_unix.go index e83c2751ed..cdd8973481 100644 --- a/components/engine/daemon/initlayer/setup_unix.go +++ b/components/engine/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/components/engine/daemon/initlayer/setup_windows.go b/components/engine/daemon/initlayer/setup_windows.go index 48a9d71aa5..2b22f58b5e 100644 --- a/components/engine/daemon/initlayer/setup_windows.go +++ b/components/engine/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/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 55a6cd8ae0..850fb76ecf 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/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,8 @@ 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 { + rootIDs, _ := daemon.idMappings.RootPair() + if err := c.SetupWorkingDirectory(rootIDs); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/components/engine/daemon/oci_solaris.go b/components/engine/daemon/oci_solaris.go index 0c757f9196..b4d39e476d 100644 --- a/components/engine/daemon/oci_solaris.go +++ b/components/engine/daemon/oci_solaris.go @@ -130,8 +130,8 @@ 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 { + rootIDs, _ := daemon.idMappings.RootPair() + if err := c.SetupWorkingDirectory(rootIDs); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/components/engine/daemon/volumes_unix.go b/components/engine/daemon/volumes_unix.go index 370eb74218..8736b7dffc 100644 --- a/components/engine/daemon/volumes_unix.go +++ b/components/engine/daemon/volumes_unix.go @@ -54,8 +54,8 @@ 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) + rootIDs, _ := daemon.idMappings.RootPair() + path, err := m.Setup(c.MountLabel, rootIDs, checkfunc) if err != nil { return nil, err } @@ -85,9 +85,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/components/engine/daemon/volumes_windows.go b/components/engine/daemon/volumes_windows.go index 76940c67df..62c9e23ac0 100644 --- a/components/engine/daemon/volumes_windows.go +++ b/components/engine/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/components/engine/daemon/workdir.go b/components/engine/daemon/workdir.go index 99a2a8ea57..90e4d709a2 100644 --- a/components/engine/daemon/workdir.go +++ b/components/engine/daemon/workdir.go @@ -16,6 +16,6 @@ func (daemon *Daemon) ContainerCreateWorkdir(cID string) error { return err } defer daemon.Unmount(container) - rootUID, rootGID := daemon.GetRemappedUIDGID() - return container.SetupWorkingDirectory(rootUID, rootGID) + rootIDs, _ := daemon.idMappings.RootPair() + return container.SetupWorkingDirectory(rootIDs) } diff --git a/components/engine/layer/layer_store.go b/components/engine/layer/layer_store.go index 5caa5d41f2..25861c6669 100644 --- a/components/engine/layer/layer_store.go +++ b/components/engine/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/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index 6bca466286..d7a3a45387 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -37,6 +37,7 @@ 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) } @@ -44,16 +45,38 @@ func MkdirAllAs(path string, mode os.FileMode, ownerUID, ownerGID int) error { // 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 +// Deprecated: Use MkdirAllAndChownNew 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) { @@ -108,26 +131,59 @@ 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 +} + +// RootPair returns a uid and gid pair for the root user +func (i *IDMappings) RootPair() (IDPair, error) { + uid, gid, err := GetRootUIDGID(i.uids, i.gids) + return IDPair{UID: uid, GID: gid}, err +} + +// 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/components/engine/pkg/idtools/idtools_unix.go b/components/engine/pkg/idtools/idtools_unix.go index 7c7e82aee2..0b28249fa8 100644 --- a/components/engine/pkg/idtools/idtools_unix.go +++ b/components/engine/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/components/engine/pkg/idtools/idtools_windows.go b/components/engine/pkg/idtools/idtools_windows.go index 49f67e78c1..8ed8353060 100644 --- a/components/engine/pkg/idtools/idtools_windows.go +++ b/components/engine/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/components/engine/plugin/manager_linux.go b/components/engine/plugin/manager_linux.go index 80fc041623..5396b15bec 100644 --- a/components/engine/plugin/manager_linux.go +++ b/components/engine/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/components/engine/volume/local/local.go b/components/engine/volume/local/local.go index 6631423bbc..43ba1e1db7 100644 --- a/components/engine/volume/local/local.go +++ b/components/engine/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/components/engine/volume/local/local_test.go b/components/engine/volume/local/local_test.go index f5a519b883..2353391aa5 100644 --- a/components/engine/volume/local/local_test.go +++ b/components/engine/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/components/engine/volume/volume.go b/components/engine/volume/volume.go index 4d761fa10c..2abfcc7b58 100644 --- a/components/engine/volume/volume.go +++ b/components/engine/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) From 5d87b0ddc99a7326ca50f75070a9274e6e629035 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 24 May 2017 11:53:41 -0400 Subject: [PATCH 2/7] Remove unused functions from archive. Signed-off-by: Daniel Nephin Upstream-commit: 967ef7e6d2bd88a5d7010863f3d7138ca61b1939 Component: engine --- components/engine/container/container_unix.go | 2 +- components/engine/daemon/archive.go | 7 +- .../daemon/archive_tarcopyoptions_unix.go | 5 +- .../engine/daemon/graphdriver/vfs/driver.go | 2 +- .../engine/distribution/registry_unit_test.go | 2 +- components/engine/layer/layer_test.go | 3 +- components/engine/pkg/archive/archive.go | 133 +++++------------- components/engine/pkg/archive/archive_test.go | 65 ++++----- .../pkg/archive/archive_windows_test.go | 2 +- .../engine/pkg/chrootarchive/archive.go | 39 +---- .../engine/pkg/chrootarchive/archive_test.go | 18 +++ components/engine/pkg/idtools/idtools.go | 11 ++ 12 files changed, 104 insertions(+), 185 deletions(-) diff --git a/components/engine/container/container_unix.go b/components/engine/container/container_unix.go index b46e100bb1..530343d5ca 100644 --- a/components/engine/container/container_unix.go +++ b/components/engine/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/components/engine/daemon/archive.go b/components/engine/daemon/archive.go index c41298df0e..79b2ae362f 100644 --- a/components/engine/daemon/archive.go +++ b/components/engine/daemon/archive.go @@ -413,12 +413,7 @@ func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decomp destExists = false } - archiver := &archive.Archiver{ - Untar: chrootarchive.Untar, - UIDMaps: daemon.idMappings.UIDs(), - GIDMaps: daemon.idMappings.GIDs(), - } - + archiver := chrootarchive.NewArchiver(daemon.idMappings) src, err := os.Stat(fullSrcPath) if err != nil { return err diff --git a/components/engine/daemon/archive_tarcopyoptions_unix.go b/components/engine/daemon/archive_tarcopyoptions_unix.go index 7b68bc463d..e7f47058a4 100644 --- a/components/engine/daemon/archive_tarcopyoptions_unix.go +++ b/components/engine/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/components/engine/daemon/graphdriver/vfs/driver.go b/components/engine/daemon/graphdriver/vfs/driver.go index 846215e810..26dc89c36d 100644 --- a/components/engine/daemon/graphdriver/vfs/driver.go +++ b/components/engine/daemon/graphdriver/vfs/driver.go @@ -15,7 +15,7 @@ import ( var ( // CopyWithTar defines the copy method to use. - CopyWithTar = chrootarchive.CopyWithTar + CopyWithTar = chrootarchive.NewArchiver(nil).CopyWithTar ) func init() { diff --git a/components/engine/distribution/registry_unit_test.go b/components/engine/distribution/registry_unit_test.go index ebe6ecad97..910061f45a 100644 --- a/components/engine/distribution/registry_unit_test.go +++ b/components/engine/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/components/engine/layer/layer_test.go b/components/engine/layer/layer_test.go index 5c4da4f9a9..56340d3012 100644 --- a/components/engine/layer/layer_test.go +++ b/components/engine/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/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index 30b3c5b36f..bf5ed85566 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/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 { @@ -496,7 +472,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 +552,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 @@ -941,7 +917,7 @@ loop: } } - if err := createTarFile(path, dest, hdr, trBuf, !options.NoLchown, options.ChownOpts, options.InUserNS); err != nil { + if err := createTarFile(path, dest, hdr, trBuf, !options.NoLchown, &options.ChownOpts, options.InUserNS); err != nil { return err } @@ -1013,23 +989,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 +1003,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 +1026,19 @@ 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) + rootIDs, err := archiver.IDMappings.RootPair() if err != nil { return err } // 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,26 +1080,24 @@ 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) + remappedRootIDs, err := archiver.IDMappings.RootPair() if 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 remappedRootIDs.UID != hdr.Uid { + hdr.Uid, err = archiver.IDMappings.UIDToHost(hdr.Uid) if err != nil { return err } - hdr.Uid = xUID } - if remappedRootGID != hdr.Gid { - xGID, err := idtools.ToHost(hdr.Gid, archiver.GIDMaps) + if remappedRootIDs.GID != hdr.Gid { + hdr.Gid, err = archiver.IDMappings.GIDToHost(hdr.Gid) if err != nil { return err } - hdr.Gid = xGID } tw := tar.NewWriter(w) @@ -1176,18 +1123,6 @@ 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) -} - // cmdStream executes a command, and returns its stdout as a stream. // If the command fails to run or doesn't complete successfully, an error // will be returned, including anything written on stderr. diff --git a/components/engine/pkg/archive/archive_test.go b/components/engine/pkg/archive/archive_test.go index e351a0455b..e85878fd0f 100644 --- a/components/engine/pkg/archive/archive_test.go +++ b/components/engine/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/components/engine/pkg/archive/archive_windows_test.go b/components/engine/pkg/archive/archive_windows_test.go index 9dd937f296..685e114baf 100644 --- a/components/engine/pkg/archive/archive_windows_test.go +++ b/components/engine/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/components/engine/pkg/chrootarchive/archive.go b/components/engine/pkg/chrootarchive/archive.go index a7814f5b90..d996ef774a 100644 --- a/components/engine/pkg/chrootarchive/archive.go +++ b/components/engine/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") } @@ -65,33 +70,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/components/engine/pkg/chrootarchive/archive_test.go b/components/engine/pkg/chrootarchive/archive_test.go index 80e54a0edc..4780f3be08 100644 --- a/components/engine/pkg/chrootarchive/archive_test.go +++ b/components/engine/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/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index d7a3a45387..39c9805df2 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -118,6 +118,7 @@ func ToContainer(hostID int, idMap []IDMap) (int, error) { // 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 # +// Depercated: use IDMappings.UIDToHost and IDMappings.GIDToHost func ToHost(contID int, idMap []IDMap) (int, error) { if idMap == nil { return contID, nil @@ -174,6 +175,16 @@ func (i *IDMappings) RootPair() (IDPair, error) { return IDPair{UID: uid, GID: gid}, err } +// UIDToHost returns the host UID for the container uid +func (i *IDMappings) UIDToHost(uid int) (int, error) { + return ToHost(uid, i.uids) +} + +// GIDToHost returns the host GID for the container gid +func (i *IDMappings) GIDToHost(gid int) (int, error) { + return ToHost(gid, i.gids) +} + // UIDs return the UID mapping // TODO: remove this once everything has been refactored to use pairs func (i *IDMappings) UIDs() []IDMap { From c41fc33a7a22e083cc3da09ff1cbde78c0d9417e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 24 May 2017 14:10:15 -0400 Subject: [PATCH 3/7] Convert tarAppender to the newIDMappings. Signed-off-by: Daniel Nephin Upstream-commit: 5672eeb5e06fe96451f36f35be7cfa18a4cf5063 Component: engine --- components/engine/pkg/archive/archive.go | 35 +++++++++++++----------- components/engine/pkg/archive/changes.go | 9 ++---- components/engine/pkg/idtools/idtools.go | 25 +++++++++++++++-- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index bf5ed85566..0418829c26 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -345,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 @@ -356,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) { @@ -404,21 +412,19 @@ 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) { + if !strings.HasPrefix(filepath.Base(hdr.Name), WhiteoutPrefix) && !ta.IDMappings.Empty() { uid, gid, err := getFileUIDGID(fi.Sys()) if err != nil { return err } - xUID, err := idtools.ToContainer(uid, ta.UIDMaps) + hdr.Uid, err = ta.IDMappings.UIDToContainer(uid) if err != nil { return err } - xGID, err := idtools.ToContainer(gid, ta.GIDMaps) + hdr.Gid, err = ta.IDMappings.GIDToContainer(gid) if err != nil { return err } - hdr.Uid = xUID - hdr.Gid = xGID } if ta.WhiteoutConverter != nil { @@ -640,14 +646,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. diff --git a/components/engine/pkg/archive/changes.go b/components/engine/pkg/archive/changes.go index ca2c0ca1bf..5ca39b7215 100644 --- a/components/engine/pkg/archive/changes.go +++ b/components/engine/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/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index 39c9805df2..5488a8be50 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -99,10 +99,10 @@ func GetRootUIDGID(uidMap, gidMap []IDMap) (int, int, error) { 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 } @@ -169,6 +169,12 @@ func NewIDMappings(username, groupname string) (*IDMappings, error) { }, 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 func (i *IDMappings) RootPair() (IDPair, error) { uid, gid, err := GetRootUIDGID(i.uids, i.gids) @@ -185,6 +191,21 @@ func (i *IDMappings) GIDToHost(gid int) (int, error) { return ToHost(gid, i.gids) } +// UIDToContainer returns the container UID for the host uid +func (i *IDMappings) UIDToContainer(uid int) (int, error) { + return toContainer(uid, i.uids) +} + +// GIDToContainer returns the container GID for the host gid +func (i *IDMappings) GIDToContainer(gid int) (int, error) { + return toContainer(gid, i.gids) +} + +// 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 { From 03637cd7aab516f9c6fca510343080c33496c8db Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 24 May 2017 16:25:13 -0400 Subject: [PATCH 4/7] Fix vfs unit test and port VFS to the new IDMappings The test was failing because TarOptions was using a non-pointer for ChownOpts, which meant the check for nil was never true, and createTarFile was never using the hdr.UID/GID Signed-off-by: Daniel Nephin Upstream-commit: acdbc285e29ddd92e7a1cc99daf8b16502204d2e Component: engine --- .../daemon/archive_tarcopyoptions_unix.go | 2 +- .../graphdriver/devmapper/devmapper_test.go | 44 ++++++- .../graphdriver/graphtest/graphtest_unix.go | 52 +++----- .../graphdriver/graphtest/testutil_unix.go | 118 ++++-------------- .../engine/daemon/graphdriver/vfs/driver.go | 26 ++-- components/engine/pkg/archive/archive.go | 4 +- 6 files changed, 93 insertions(+), 153 deletions(-) diff --git a/components/engine/daemon/archive_tarcopyoptions_unix.go b/components/engine/daemon/archive_tarcopyoptions_unix.go index e7f47058a4..83e6fd9e17 100644 --- a/components/engine/daemon/archive_tarcopyoptions_unix.go +++ b/components/engine/daemon/archive_tarcopyoptions_unix.go @@ -20,6 +20,6 @@ func (daemon *Daemon) tarCopyOptions(container *container.Container, noOverwrite return &archive.TarOptions{ NoOverwriteDirNonDir: noOverwriteDirNonDir, - ChownOpts: idtools.IDPair{UID: user.Uid, GID: user.Gid}, + ChownOpts: &idtools.IDPair{UID: user.Uid, GID: user.Gid}, }, nil } diff --git a/components/engine/daemon/graphdriver/devmapper/devmapper_test.go b/components/engine/daemon/graphdriver/devmapper/devmapper_test.go index c5be97ae3f..7501397fdc 100644 --- a/components/engine/daemon/graphdriver/devmapper/devmapper_test.go +++ b/components/engine/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/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go b/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go index 6e952de78b..6852ca9f4c 100644 --- a/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/components/engine/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/components/engine/daemon/graphdriver/graphtest/testutil_unix.go b/components/engine/daemon/graphdriver/graphtest/testutil_unix.go index 49b0c2cc35..63a934176e 100644 --- a/components/engine/daemon/graphdriver/graphtest/testutil_unix.go +++ b/components/engine/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/components/engine/daemon/graphdriver/vfs/driver.go b/components/engine/daemon/graphdriver/vfs/driver.go index 26dc89c36d..f1b8b8661c 100644 --- a/components/engine/daemon/graphdriver/vfs/driver.go +++ b/components/engine/daemon/graphdriver/vfs/driver.go @@ -9,7 +9,6 @@ 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" ) @@ -26,15 +25,14 @@ 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) + rootIDs, err := d.idMappings.RootPair() if err != nil { return nil, err } - if err := idtools.MkdirAllAs(home, 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(home, 0700, rootIDs); err != nil { return nil, err } return graphdriver.NewNaiveDiffDriver(d, uidMaps, gidMaps), nil @@ -45,9 +43,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 +79,14 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error { } dir := d.dir(id) - rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps) + rootIDs, err := d.idMappings.RootPair() if err != nil { return err } - if err := idtools.MkdirAllAs(filepath.Dir(dir), 0700, rootUID, rootGID); err != nil { + if err := idtools.MkdirAllAndChown(filepath.Dir(dir), 0700, rootIDs); 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 +100,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/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index 0418829c26..59154a4bde 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -39,7 +39,7 @@ type ( NoLchown bool UIDMaps []idtools.IDMap GIDMaps []idtools.IDMap - ChownOpts idtools.IDPair + 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 @@ -920,7 +920,7 @@ loop: } } - if err := createTarFile(path, dest, hdr, trBuf, !options.NoLchown, &options.ChownOpts, options.InUserNS); err != nil { + if err := createTarFile(path, dest, hdr, trBuf, !options.NoLchown, options.ChownOpts, options.InUserNS); err != nil { return err } From f40a1d3270858fb0ae34fae09db683055d946fdb Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 31 May 2017 17:18:04 -0400 Subject: [PATCH 5/7] Remove ToHost and replace it with IDMappings.ToHost Signed-off-by: Daniel Nephin Upstream-commit: df248d31d9d61342575fc1b5d3d848f0b282bbc5 Component: engine --- components/engine/pkg/archive/archive.go | 59 ++++------------ components/engine/pkg/archive/archive_unix.go | 7 +- .../engine/pkg/archive/archive_windows.go | 5 +- components/engine/pkg/archive/diff.go | 28 ++------ components/engine/pkg/idtools/idtools.go | 68 ++++++++++--------- 5 files changed, 60 insertions(+), 107 deletions(-) diff --git a/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index 59154a4bde..fe09cdec4a 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -413,15 +413,11 @@ func (ta *tarAppender) addTarFile(path, name string) error { //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.IDMappings.Empty() { - uid, gid, err := getFileUIDGID(fi.Sys()) + fileIDPair, err := getFileUIDGID(fi.Sys()) if err != nil { return err } - hdr.Uid, err = ta.IDMappings.UIDToContainer(uid) - if err != nil { - return err - } - hdr.Gid, err = ta.IDMappings.GIDToContainer(gid) + hdr.Uid, hdr.Gid, err = ta.IDMappings.ToContainer(fileIDPair) if err != nil { return err } @@ -806,7 +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) + idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) + rootIDs, err := idMappings.RootPair() if err != nil { return err } @@ -843,7 +840,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 } @@ -888,26 +885,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 { @@ -1083,26 +1062,10 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) { hdr.Name = filepath.Base(dst) hdr.Mode = int64(chmodTarEntry(os.FileMode(hdr.Mode))) - remappedRootIDs, err := archiver.IDMappings.RootPair() - 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 remappedRootIDs.UID != hdr.Uid { - hdr.Uid, err = archiver.IDMappings.UIDToHost(hdr.Uid) - if err != nil { - return err - } - } - if remappedRootIDs.GID != hdr.Gid { - hdr.Gid, err = archiver.IDMappings.GIDToHost(hdr.Gid) - if err != nil { - return err - } - } - tw := tar.NewWriter(w) defer tw.Close() if err := tw.WriteHeader(hdr); err != nil { @@ -1126,6 +1089,12 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) { return err } +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. // If the command fails to run or doesn't complete successfully, an error // will be returned, including anything written on stderr. diff --git a/components/engine/pkg/archive/archive_unix.go b/components/engine/pkg/archive/archive_unix.go index 68d3c97d27..33ee88766f 100644 --- a/components/engine/pkg/archive/archive_unix.go +++ b/components/engine/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/components/engine/pkg/archive/archive_windows.go b/components/engine/pkg/archive/archive_windows.go index 9c7094738d..a22410c039 100644 --- a/components/engine/pkg/archive/archive_windows.go +++ b/components/engine/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/components/engine/pkg/archive/diff.go b/components/engine/pkg/archive/diff.go index 2292193942..d20854e2a2 100644 --- a/components/engine/pkg/archive/diff.go +++ b/components/engine/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/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index 5488a8be50..cd5ca51dfb 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -80,21 +80,13 @@ func MkdirAllAndChownNew(path string, mode os.FileMode, ids IDPair) error { // 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 } @@ -115,11 +107,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 # -// Depercated: use IDMappings.UIDToHost and IDMappings.GIDToHost -func ToHost(contID int, idMap []IDMap) (int, error) { +func toHost(contID int, idMap []IDMap) (int, error) { if idMap == nil { return contID, nil } @@ -181,24 +172,35 @@ func (i *IDMappings) RootPair() (IDPair, error) { return IDPair{UID: uid, GID: gid}, err } -// UIDToHost returns the host UID for the container uid -func (i *IDMappings) UIDToHost(uid int) (int, error) { - return ToHost(uid, i.uids) +// 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) { + target, err := i.RootPair() + if err != nil { + return IDPair{}, err + } + + 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 } -// GIDToHost returns the host GID for the container gid -func (i *IDMappings) GIDToHost(gid int) (int, error) { - return ToHost(gid, i.gids) -} - -// UIDToContainer returns the container UID for the host uid -func (i *IDMappings) UIDToContainer(uid int) (int, error) { - return toContainer(uid, i.uids) -} - -// GIDToContainer returns the container GID for the host gid -func (i *IDMappings) GIDToContainer(gid int) (int, error) { - return toContainer(gid, i.gids) +// 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 From b35fc7f2687873d71c5ec079569fc2ff111b28df Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 31 May 2017 17:36:48 -0400 Subject: [PATCH 6/7] Remove MkdirAllNewAs and update tests. Signed-off-by: Daniel Nephin Upstream-commit: 6150ebf7b483197f4b8755df60e750b6410e95ca Component: engine --- .../engine/libcontainerd/client_unix.go | 4 +- .../engine/pkg/chrootarchive/archive.go | 5 +- components/engine/pkg/idtools/idtools.go | 8 --- .../engine/pkg/idtools/idtools_unix_test.go | 54 +++++++------------ 4 files changed, 23 insertions(+), 48 deletions(-) diff --git a/components/engine/libcontainerd/client_unix.go b/components/engine/libcontainerd/client_unix.go index 906026024d..456e21fceb 100644 --- a/components/engine/libcontainerd/client_unix.go +++ b/components/engine/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/components/engine/pkg/chrootarchive/archive.go b/components/engine/pkg/chrootarchive/archive.go index d996ef774a..61522c75fc 100644 --- a/components/engine/pkg/chrootarchive/archive.go +++ b/components/engine/pkg/chrootarchive/archive.go @@ -46,14 +46,15 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions options.ExcludePatterns = []string{} } - rootUID, rootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps) + idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) + rootIDs, err := idMappings.RootPair() if err != nil { return err } 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 } } diff --git a/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index cd5ca51dfb..36cd688c7d 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -42,14 +42,6 @@ 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 -// Deprecated: Use MkdirAllAndChownNew -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 diff --git a/components/engine/pkg/idtools/idtools_unix_test.go b/components/engine/pkg/idtools/idtools_unix_test.go index 540d3079ee..31522a547d 100644 --- a/components/engine/pkg/idtools/idtools_unix_test.go +++ b/components/engine/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) { From 583893964eeeb09807cbe914bd3955c9842b00d4 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 31 May 2017 17:56:23 -0400 Subject: [PATCH 7/7] Remove error return from RootPair There is no case which would resolve in this error. The root user always exists, and if the id maps are empty, the default value of 0 is correct. Signed-off-by: Daniel Nephin Upstream-commit: 93fbdb69acf9248283a91a1c5c6ea24711c26eda Component: engine --- components/engine/daemon/archive.go | 2 +- .../daemon/container_operations_unix.go | 6 +++--- components/engine/daemon/create.go | 5 +---- components/engine/daemon/create_unix.go | 2 +- components/engine/daemon/daemon.go | 19 +++---------------- .../engine/daemon/graphdriver/vfs/driver.go | 10 ++-------- components/engine/daemon/info.go | 2 +- components/engine/daemon/oci_linux.go | 3 +-- components/engine/daemon/oci_solaris.go | 3 +-- components/engine/daemon/volumes_unix.go | 5 ++--- components/engine/daemon/workdir.go | 3 +-- components/engine/pkg/archive/archive.go | 10 ++-------- .../engine/pkg/chrootarchive/archive.go | 5 +---- components/engine/pkg/idtools/idtools.go | 16 ++++++++-------- 14 files changed, 28 insertions(+), 63 deletions(-) diff --git a/components/engine/daemon/archive.go b/components/engine/daemon/archive.go index 79b2ae362f..3fb75bbb22 100644 --- a/components/engine/daemon/archive.go +++ b/components/engine/daemon/archive.go @@ -375,7 +375,7 @@ func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decomp destExists := true destDir := false - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() // Work in daemon-local OS specific file paths destPath = filepath.FromSlash(destPath) diff --git a/components/engine/daemon/container_operations_unix.go b/components/engine/daemon/container_operations_unix.go index 5ae7a68f0b..99d17e41b5 100644 --- a/components/engine/daemon/container_operations_unix.go +++ b/components/engine/daemon/container_operations_unix.go @@ -109,7 +109,7 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { } c.ShmPath = "/dev/shm" } else { - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() if !c.HasMountFor("/dev/shm") { shmPath, err := c.ShmResourcePath() if err != nil { @@ -147,7 +147,7 @@ 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 - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() // create tmpfs if err := idtools.MkdirAllAndChown(localMountPath, 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating secret local mount path") @@ -232,7 +232,7 @@ 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 - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() // create tmpfs if err := idtools.MkdirAllAndChown(localPath, 0700, rootIDs); err != nil { return errors.Wrap(err, "error creating config dir") diff --git a/components/engine/daemon/create.go b/components/engine/daemon/create.go index 52b4b0e6fa..697886274f 100644 --- a/components/engine/daemon/create.go +++ b/components/engine/daemon/create.go @@ -117,10 +117,7 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( return nil, err } - rootIDs, err := daemon.idMappings.RootPair() - if err != nil { - return nil, err - } + rootIDs := daemon.idMappings.RootPair() if err := idtools.MkdirAndChown(container.Root, 0700, rootIDs); err != nil { return nil, err } diff --git a/components/engine/daemon/create_unix.go b/components/engine/daemon/create_unix.go index 27471e7556..2501a3374a 100644 --- a/components/engine/daemon/create_unix.go +++ b/components/engine/daemon/create_unix.go @@ -22,7 +22,7 @@ func (daemon *Daemon) createContainerPlatformSpecificSettings(container *contain } defer daemon.Unmount(container) - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() if err := container.SetupWorkingDirectory(rootIDs); err != nil { return err } diff --git a/components/engine/daemon/daemon.go b/components/engine/daemon/daemon.go index 3e8b3106a2..4b78d01370 100644 --- a/components/engine/daemon/daemon.go +++ b/components/engine/daemon/daemon.go @@ -527,11 +527,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe if err != nil { return nil, err } - rootIDs, err := idMappings.RootPair() - if err != nil { - return nil, err - } - + rootIDs := idMappings.RootPair() if err := setupDaemonProcess(config); err != nil { return nil, err } @@ -994,7 +990,7 @@ func prepareTempDir(rootDir string, rootIDs idtools.IDPair) (string, error) { } func (daemon *Daemon) setupInitLayer(initPath string) error { - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() return initlayer.Setup(initPath, rootIDs) } @@ -1157,14 +1153,5 @@ func CreateDaemonRoot(config *config.Config) error { if err != nil { return err } - rootIDs, err := idMappings.RootPair() - if err != nil { - return err - } - - if err := setupDaemonRoot(config, realRoot, rootIDs); err != nil { - return err - } - - return nil + return setupDaemonRoot(config, realRoot, idMappings.RootPair()) } diff --git a/components/engine/daemon/graphdriver/vfs/driver.go b/components/engine/daemon/graphdriver/vfs/driver.go index f1b8b8661c..15a4de3606 100644 --- a/components/engine/daemon/graphdriver/vfs/driver.go +++ b/components/engine/daemon/graphdriver/vfs/driver.go @@ -28,10 +28,7 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap home: home, idMappings: idtools.NewIDMappingsFromMaps(uidMaps, gidMaps), } - rootIDs, err := d.idMappings.RootPair() - if err != nil { - return nil, err - } + rootIDs := d.idMappings.RootPair() if err := idtools.MkdirAllAndChown(home, 0700, rootIDs); err != nil { return nil, err } @@ -79,10 +76,7 @@ func (d *Driver) Create(id, parent string, opts *graphdriver.CreateOpts) error { } dir := d.dir(id) - rootIDs, err := d.idMappings.RootPair() - if err != nil { - return err - } + rootIDs := d.idMappings.RootPair() if err := idtools.MkdirAllAndChown(filepath.Dir(dir), 0700, rootIDs); err != nil { return err } diff --git a/components/engine/daemon/info.go b/components/engine/daemon/info.go index 3cadea791e..3c28cdf7f1 100644 --- a/components/engine/daemon/info.go +++ b/components/engine/daemon/info.go @@ -72,7 +72,7 @@ func (daemon *Daemon) SystemInfo() (*types.Info, error) { if selinuxEnabled() { securityOptions = append(securityOptions, "name=selinux") } - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() if rootIDs.UID != 0 || rootIDs.GID != 0 { securityOptions = append(securityOptions, "name=userns") } diff --git a/components/engine/daemon/oci_linux.go b/components/engine/daemon/oci_linux.go index 850fb76ecf..6d74301a05 100644 --- a/components/engine/daemon/oci_linux.go +++ b/components/engine/daemon/oci_linux.go @@ -611,8 +611,7 @@ func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) Path: c.BaseFS, Readonly: c.HostConfig.ReadonlyRootfs, } - rootIDs, _ := daemon.idMappings.RootPair() - if err := c.SetupWorkingDirectory(rootIDs); err != nil { + if err := c.SetupWorkingDirectory(daemon.idMappings.RootPair()); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/components/engine/daemon/oci_solaris.go b/components/engine/daemon/oci_solaris.go index b4d39e476d..610efe10a1 100644 --- a/components/engine/daemon/oci_solaris.go +++ b/components/engine/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, } - rootIDs, _ := daemon.idMappings.RootPair() - if err := c.SetupWorkingDirectory(rootIDs); err != nil { + if err := c.SetupWorkingDirectory(daemon.idMappings.RootPair()); err != nil { return err } cwd := c.Config.WorkingDir diff --git a/components/engine/daemon/volumes_unix.go b/components/engine/daemon/volumes_unix.go index 8736b7dffc..d91100f1c9 100644 --- a/components/engine/daemon/volumes_unix.go +++ b/components/engine/daemon/volumes_unix.go @@ -54,8 +54,7 @@ func (daemon *Daemon) setupMounts(c *container.Container) ([]container.Mount, er return nil } - rootIDs, _ := daemon.idMappings.RootPair() - path, err := m.Setup(c.MountLabel, rootIDs, checkfunc) + path, err := m.Setup(c.MountLabel, daemon.idMappings.RootPair(), checkfunc) if err != nil { return nil, err } @@ -85,7 +84,7 @@ 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) - rootIDs, _ := daemon.idMappings.RootPair() + rootIDs := daemon.idMappings.RootPair() for _, mount := range netMounts { if err := os.Chown(mount.Source, rootIDs.UID, rootIDs.GID); err != nil { return nil, err diff --git a/components/engine/daemon/workdir.go b/components/engine/daemon/workdir.go index 90e4d709a2..6360f24138 100644 --- a/components/engine/daemon/workdir.go +++ b/components/engine/daemon/workdir.go @@ -16,6 +16,5 @@ func (daemon *Daemon) ContainerCreateWorkdir(cID string) error { return err } defer daemon.Unmount(container) - rootIDs, _ := daemon.idMappings.RootPair() - return container.SetupWorkingDirectory(rootIDs) + return container.SetupWorkingDirectory(daemon.idMappings.RootPair()) } diff --git a/components/engine/pkg/archive/archive.go b/components/engine/pkg/archive/archive.go index fe09cdec4a..5fb774d602 100644 --- a/components/engine/pkg/archive/archive.go +++ b/components/engine/pkg/archive/archive.go @@ -803,10 +803,7 @@ func Unpack(decompressedArchive io.Reader, dest string, options *TarOptions) err var dirs []*tar.Header idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) - rootIDs, err := idMappings.RootPair() - if err != nil { - return err - } + rootIDs := idMappings.RootPair() whiteoutConverter := getWhiteoutConverter(options.WhiteoutFormat) // Iterate through the files in the archive. @@ -1008,10 +1005,7 @@ 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 - rootIDs, err := archiver.IDMappings.RootPair() - 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.MkdirAllAndChownNew(dst, 0755, rootIDs); err != nil { diff --git a/components/engine/pkg/chrootarchive/archive.go b/components/engine/pkg/chrootarchive/archive.go index 61522c75fc..7604418767 100644 --- a/components/engine/pkg/chrootarchive/archive.go +++ b/components/engine/pkg/chrootarchive/archive.go @@ -47,10 +47,7 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions } idMappings := idtools.NewIDMappingsFromMaps(options.UIDMaps, options.GIDMaps) - rootIDs, err := idMappings.RootPair() - if err != nil { - return err - } + rootIDs := idMappings.RootPair() dest = filepath.Clean(dest) if _, err := os.Stat(dest); os.IsNotExist(err) { diff --git a/components/engine/pkg/idtools/idtools.go b/components/engine/pkg/idtools/idtools.go index 36cd688c7d..68a072db22 100644 --- a/components/engine/pkg/idtools/idtools.go +++ b/components/engine/pkg/idtools/idtools.go @@ -158,19 +158,19 @@ func NewIDMappingsFromMaps(uids []IDMap, gids []IDMap) *IDMappings { return &IDMappings{uids: uids, gids: gids} } -// RootPair returns a uid and gid pair for the root user -func (i *IDMappings) RootPair() (IDPair, error) { - uid, gid, err := GetRootUIDGID(i.uids, i.gids) - return IDPair{UID: uid, GID: gid}, err +// 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) { - target, err := i.RootPair() - if err != nil { - return IDPair{}, err - } + var err error + target := i.RootPair() if pair.UID != target.UID { target.UID, err = toHost(pair.UID, i.uids)