From 583893964eeeb09807cbe914bd3955c9842b00d4 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 31 May 2017 17:56:23 -0400 Subject: [PATCH] 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)