From 6150ebf7b483197f4b8755df60e750b6410e95ca Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 31 May 2017 17:36:48 -0400 Subject: [PATCH] Remove MkdirAllNewAs and update tests. Signed-off-by: Daniel Nephin --- libcontainerd/client_unix.go | 4 +-- pkg/chrootarchive/archive.go | 5 +-- pkg/idtools/idtools.go | 8 ----- pkg/idtools/idtools_unix_test.go | 54 +++++++++++--------------------- 4 files changed, 23 insertions(+), 48 deletions(-) diff --git a/libcontainerd/client_unix.go b/libcontainerd/client_unix.go index 906026024d..456e21fceb 100644 --- a/libcontainerd/client_unix.go +++ b/libcontainerd/client_unix.go @@ -34,7 +34,7 @@ func (clnt *client) prepareBundleDir(uid, gid int) (string, error) { } if os.IsNotExist(err) || fi.Mode()&1 == 0 { p = fmt.Sprintf("%s.%d.%d", p, uid, gid) - if err := idtools.MkdirAs(p, 0700, uid, gid); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAndChown(p, 0700, idtools.IDPair{uid, gid}); err != nil && !os.IsExist(err) { return "", err } } @@ -71,7 +71,7 @@ func (clnt *client) Create(containerID string, checkpoint string, checkpointDir } }() - if err := idtools.MkdirAllAs(container.dir, 0700, uid, gid); err != nil && !os.IsExist(err) { + if err := idtools.MkdirAllAndChown(container.dir, 0700, idtools.IDPair{uid, gid}); err != nil && !os.IsExist(err) { return err } diff --git a/pkg/chrootarchive/archive.go b/pkg/chrootarchive/archive.go index d996ef774a..61522c75fc 100644 --- a/pkg/chrootarchive/archive.go +++ b/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/pkg/idtools/idtools.go b/pkg/idtools/idtools.go index cd5ca51dfb..36cd688c7d 100644 --- a/pkg/idtools/idtools.go +++ b/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/pkg/idtools/idtools_unix_test.go b/pkg/idtools/idtools_unix_test.go index 540d3079ee..31522a547d 100644 --- a/pkg/idtools/idtools_unix_test.go +++ b/pkg/idtools/idtools_unix_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "syscall" "testing" + + "github.com/stretchr/testify/require" ) type node struct { @@ -76,12 +78,9 @@ func TestMkdirAllAs(t *testing.T) { } } -func TestMkdirAllNewAs(t *testing.T) { - +func TestMkdirAllAndChownNew(t *testing.T) { dirName, err := ioutil.TempDir("", "mkdirnew") - if err != nil { - t.Fatalf("Couldn't create temp dir: %v", err) - } + require.NoError(t, err) defer os.RemoveAll(dirName) testTree := map[string]node{ @@ -91,49 +90,32 @@ func TestMkdirAllNewAs(t *testing.T) { "lib/x86_64": {45, 45}, "lib/x86_64/share": {1, 1}, } - - if err := buildTree(dirName, testTree); err != nil { - t.Fatal(err) - } + require.NoError(t, buildTree(dirName, testTree)) // test adding a directory to a pre-existing dir; only the new dir is owned by the uid/gid - if err := MkdirAllNewAs(filepath.Join(dirName, "usr", "share"), 0755, 99, 99); err != nil { - t.Fatal(err) - } + err = MkdirAllAndChownNew(filepath.Join(dirName, "usr", "share"), 0755, IDPair{99, 99}) + require.NoError(t, err) + testTree["usr/share"] = node{99, 99} verifyTree, err := readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, compareTrees(testTree, verifyTree)) // test 2-deep new directories--both should be owned by the uid/gid pair - if err := MkdirAllNewAs(filepath.Join(dirName, "lib", "some", "other"), 0755, 101, 101); err != nil { - t.Fatal(err) - } + err = MkdirAllAndChownNew(filepath.Join(dirName, "lib", "some", "other"), 0755, IDPair{101, 101}) + require.NoError(t, err) testTree["lib/some"] = node{101, 101} testTree["lib/some/other"] = node{101, 101} verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, compareTrees(testTree, verifyTree)) // test a directory that already exists; should NOT be chowned - if err := MkdirAllNewAs(filepath.Join(dirName, "usr"), 0755, 102, 102); err != nil { - t.Fatal(err) - } + err = MkdirAllAndChownNew(filepath.Join(dirName, "usr"), 0755, IDPair{102, 102}) + require.NoError(t, err) verifyTree, err = readTree(dirName, "") - if err != nil { - t.Fatal(err) - } - if err := compareTrees(testTree, verifyTree); err != nil { - t.Fatal(err) - } + require.NoError(t, err) + require.NoError(t, compareTrees(testTree, verifyTree)) } func TestMkdirAs(t *testing.T) {