diff --git a/daemon/archive_tarcopyoptions_unix.go b/daemon/archive_tarcopyoptions_unix.go index e7f47058a4..83e6fd9e17 100644 --- a/daemon/archive_tarcopyoptions_unix.go +++ b/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/daemon/graphdriver/devmapper/devmapper_test.go b/daemon/graphdriver/devmapper/devmapper_test.go index c5be97ae3f..7501397fdc 100644 --- a/daemon/graphdriver/devmapper/devmapper_test.go +++ b/daemon/graphdriver/devmapper/devmapper_test.go @@ -4,6 +4,8 @@ package devmapper import ( "fmt" + "os" + "syscall" "testing" "time" @@ -17,11 +19,51 @@ func init() { defaultMetaDataLoopbackSize = 200 * 1024 * 1024 defaultBaseFsSize = 300 * 1024 * 1024 defaultUdevSyncOverride = true - if err := graphtest.InitLoopbacks(); err != nil { + if err := initLoopbacks(); err != nil { panic(err) } } +// initLoopbacks ensures that the loopback devices are properly created within +// the system running the device mapper tests. +func initLoopbacks() error { + statT, err := getBaseLoopStats() + if err != nil { + return err + } + // create at least 8 loopback files, ya, that is a good number + for i := 0; i < 8; i++ { + loopPath := fmt.Sprintf("/dev/loop%d", i) + // only create new loopback files if they don't exist + if _, err := os.Stat(loopPath); err != nil { + if mkerr := syscall.Mknod(loopPath, + uint32(statT.Mode|syscall.S_IFBLK), int((7<<8)|(i&0xff)|((i&0xfff00)<<12))); mkerr != nil { + return mkerr + } + os.Chown(loopPath, int(statT.Uid), int(statT.Gid)) + } + } + return nil +} + +// getBaseLoopStats inspects /dev/loop0 to collect uid,gid, and mode for the +// loop0 device on the system. If it does not exist we assume 0,0,0660 for the +// stat data +func getBaseLoopStats() (*syscall.Stat_t, error) { + loop0, err := os.Stat("/dev/loop0") + if err != nil { + if os.IsNotExist(err) { + return &syscall.Stat_t{ + Uid: 0, + Gid: 0, + Mode: 0660, + }, nil + } + return nil, err + } + return loop0.Sys().(*syscall.Stat_t), nil +} + // This avoids creating a new driver for each test if all tests are run // Make sure to put new tests between TestDevmapperSetup and TestDevmapperTeardown func TestDevmapperSetup(t *testing.T) { diff --git a/daemon/graphdriver/graphtest/graphtest_unix.go b/daemon/graphdriver/graphtest/graphtest_unix.go index 6e952de78b..6852ca9f4c 100644 --- a/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/daemon/graphdriver/graphtest/graphtest_unix.go @@ -16,6 +16,8 @@ import ( "github.com/docker/docker/daemon/graphdriver" "github.com/docker/docker/pkg/stringid" "github.com/docker/go-units" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) var ( @@ -33,14 +35,9 @@ type Driver struct { func newDriver(t testing.TB, name string, options []string) *Driver { root, err := ioutil.TempDir("", "docker-graphtest-") - if err != nil { - t.Fatal(err) - } - - if err := os.MkdirAll(root, 0755); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, os.MkdirAll(root, 0755)) d, err := graphdriver.GetDriver(name, nil, graphdriver.Options{DriverOptions: options, Root: root}) if err != nil { t.Logf("graphdriver: %v\n", err) @@ -86,14 +83,11 @@ func DriverTestCreateEmpty(t testing.TB, drivername string, driverOptions ...str driver := GetDriver(t, drivername, driverOptions...) defer PutDriver(t) - if err := driver.Create("empty", "", nil); err != nil { - t.Fatal(err) - } + err := driver.Create("empty", "", nil) + require.NoError(t, err) defer func() { - if err := driver.Remove("empty"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("empty")) }() if !driver.Exists("empty") { @@ -101,21 +95,14 @@ func DriverTestCreateEmpty(t testing.TB, drivername string, driverOptions ...str } dir, err := driver.Get("empty", "") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) verifyFile(t, dir, 0755|os.ModeDir, 0, 0) // Verify that the directory is empty fis, err := readDir(dir) - if err != nil { - t.Fatal(err) - } - - if len(fis) != 0 { - t.Fatal("New directory not empty") - } + require.NoError(t, err) + assert.Len(t, fis, 0) driver.Put("empty") } @@ -127,9 +114,7 @@ func DriverTestCreateBase(t testing.TB, drivername string, driverOptions ...stri createBase(t, driver, "Base") defer func() { - if err := driver.Remove("Base"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("Base")) }() verifyBase(t, driver, "Base") } @@ -140,21 +125,14 @@ func DriverTestCreateSnap(t testing.TB, drivername string, driverOptions ...stri defer PutDriver(t) createBase(t, driver, "Base") - defer func() { - if err := driver.Remove("Base"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("Base")) }() - if err := driver.Create("Snap", "Base", nil); err != nil { - t.Fatal(err) - } - + err := driver.Create("Snap", "Base", nil) + require.NoError(t, err) defer func() { - if err := driver.Remove("Snap"); err != nil { - t.Fatal(err) - } + require.NoError(t, driver.Remove("Snap")) }() verifyBase(t, driver, "Snap") diff --git a/daemon/graphdriver/graphtest/testutil_unix.go b/daemon/graphdriver/graphtest/testutil_unix.go index 49b0c2cc35..63a934176e 100644 --- a/daemon/graphdriver/graphtest/testutil_unix.go +++ b/daemon/graphdriver/graphtest/testutil_unix.go @@ -3,7 +3,6 @@ package graphtest import ( - "fmt" "io/ioutil" "os" "path" @@ -11,81 +10,24 @@ import ( "testing" "github.com/docker/docker/daemon/graphdriver" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -// InitLoopbacks ensures that the loopback devices are properly created within -// the system running the device mapper tests. -func InitLoopbacks() error { - statT, err := getBaseLoopStats() - if err != nil { - return err - } - // create at least 8 loopback files, ya, that is a good number - for i := 0; i < 8; i++ { - loopPath := fmt.Sprintf("/dev/loop%d", i) - // only create new loopback files if they don't exist - if _, err := os.Stat(loopPath); err != nil { - if mkerr := syscall.Mknod(loopPath, - uint32(statT.Mode|syscall.S_IFBLK), int((7<<8)|(i&0xff)|((i&0xfff00)<<12))); mkerr != nil { - return mkerr - } - os.Chown(loopPath, int(statT.Uid), int(statT.Gid)) - } - } - return nil -} - -// getBaseLoopStats inspects /dev/loop0 to collect uid,gid, and mode for the -// loop0 device on the system. If it does not exist we assume 0,0,0660 for the -// stat data -func getBaseLoopStats() (*syscall.Stat_t, error) { - loop0, err := os.Stat("/dev/loop0") - if err != nil { - if os.IsNotExist(err) { - return &syscall.Stat_t{ - Uid: 0, - Gid: 0, - Mode: 0660, - }, nil - } - return nil, err - } - return loop0.Sys().(*syscall.Stat_t), nil -} - func verifyFile(t testing.TB, path string, mode os.FileMode, uid, gid uint32) { fi, err := os.Stat(path) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) - if fi.Mode()&os.ModeType != mode&os.ModeType { - t.Fatalf("Expected %s type 0x%x, got 0x%x", path, mode&os.ModeType, fi.Mode()&os.ModeType) - } - - if fi.Mode()&os.ModePerm != mode&os.ModePerm { - t.Fatalf("Expected %s mode %o, got %o", path, mode&os.ModePerm, fi.Mode()&os.ModePerm) - } - - if fi.Mode()&os.ModeSticky != mode&os.ModeSticky { - t.Fatalf("Expected %s sticky 0x%x, got 0x%x", path, mode&os.ModeSticky, fi.Mode()&os.ModeSticky) - } - - if fi.Mode()&os.ModeSetuid != mode&os.ModeSetuid { - t.Fatalf("Expected %s setuid 0x%x, got 0x%x", path, mode&os.ModeSetuid, fi.Mode()&os.ModeSetuid) - } - - if fi.Mode()&os.ModeSetgid != mode&os.ModeSetgid { - t.Fatalf("Expected %s setgid 0x%x, got 0x%x", path, mode&os.ModeSetgid, fi.Mode()&os.ModeSetgid) - } + actual := fi.Mode() + assert.Equal(t, mode&os.ModeType, actual&os.ModeType, path) + assert.Equal(t, mode&os.ModePerm, actual&os.ModePerm, path) + assert.Equal(t, mode&os.ModeSticky, actual&os.ModeSticky, path) + assert.Equal(t, mode&os.ModeSetuid, actual&os.ModeSetuid, path) + assert.Equal(t, mode&os.ModeSetgid, actual&os.ModeSetgid, path) if stat, ok := fi.Sys().(*syscall.Stat_t); ok { - if stat.Uid != uid { - t.Fatalf("%s no owned by uid %d", path, uid) - } - if stat.Gid != gid { - t.Fatalf("%s not owned by gid %d", path, gid) - } + assert.Equal(t, uid, stat.Uid, path) + assert.Equal(t, gid, stat.Gid, path) } } @@ -94,35 +36,25 @@ func createBase(t testing.TB, driver graphdriver.Driver, name string) { oldmask := syscall.Umask(0) defer syscall.Umask(oldmask) - if err := driver.CreateReadWrite(name, "", nil); err != nil { - t.Fatal(err) - } + err := driver.CreateReadWrite(name, "", nil) + require.NoError(t, err) dir, err := driver.Get(name, "") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer driver.Put(name) subdir := path.Join(dir, "a subdir") - if err := os.Mkdir(subdir, 0705|os.ModeSticky); err != nil { - t.Fatal(err) - } - if err := os.Chown(subdir, 1, 2); err != nil { - t.Fatal(err) - } + require.NoError(t, os.Mkdir(subdir, 0705|os.ModeSticky)) + require.NoError(t, os.Chown(subdir, 1, 2)) file := path.Join(dir, "a file") - if err := ioutil.WriteFile(file, []byte("Some data"), 0222|os.ModeSetuid); err != nil { - t.Fatal(err) - } + err = ioutil.WriteFile(file, []byte("Some data"), 0222|os.ModeSetuid) + require.NoError(t, err) } func verifyBase(t testing.TB, driver graphdriver.Driver, name string) { dir, err := driver.Get(name, "") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer driver.Put(name) subdir := path.Join(dir, "a subdir") @@ -131,13 +63,7 @@ func verifyBase(t testing.TB, driver graphdriver.Driver, name string) { file := path.Join(dir, "a file") verifyFile(t, file, 0222|os.ModeSetuid, 0, 0) - fis, err := readDir(dir) - if err != nil { - t.Fatal(err) - } - - if len(fis) != 2 { - t.Fatal("Unexpected files in base image") - } - + files, err := readDir(dir) + require.NoError(t, err) + assert.Len(t, files, 2) } diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 26dc89c36d..f1b8b8661c 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/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/pkg/archive/archive.go b/pkg/archive/archive.go index 0418829c26..59154a4bde 100644 --- a/pkg/archive/archive.go +++ b/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 }