From f40a1d3270858fb0ae34fae09db683055d946fdb Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 31 May 2017 17:18:04 -0400 Subject: [PATCH] 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