From d15734ec3c10eda667b716f67e18d5d86e708e3e Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Tue, 24 Apr 2018 10:26:10 -0400 Subject: [PATCH] Fix issues with running volume tests as non-root. - Volume store created dir with wrong permissions - Local volume driver hardcoded uid/gid 0 Signed-off-by: Brian Goff --- volume/local/local_test.go | 22 +++++++++------------- volume/store/store.go | 2 +- volume/store/store_test.go | 11 ----------- 3 files changed, 10 insertions(+), 25 deletions(-) diff --git a/volume/local/local_test.go b/volume/local/local_test.go index 70e435676a..541b8448aa 100644 --- a/volume/local/local_test.go +++ b/volume/local/local_test.go @@ -32,14 +32,13 @@ func TestGetAddress(t *testing.T) { func TestRemove(t *testing.T) { skip.If(t, runtime.GOOS == "windows", "FIXME: investigate why this test fails on CI") - skip.If(t, os.Getuid() != 0, "skipping test that requires root") rootDir, err := ioutil.TempDir("", "local-volume-test") if err != nil { t.Fatal(err) } defer os.RemoveAll(rootDir) - r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err := New(rootDir, idtools.IDPair{UID: os.Geteuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -75,14 +74,13 @@ func TestRemove(t *testing.T) { } func TestInitializeWithVolumes(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") rootDir, err := ioutil.TempDir("", "local-volume-test") if err != nil { t.Fatal(err) } defer os.RemoveAll(rootDir) - r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err := New(rootDir, idtools.IDPair{UID: os.Geteuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -92,7 +90,7 @@ func TestInitializeWithVolumes(t *testing.T) { t.Fatal(err) } - r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err = New(rootDir, idtools.IDPair{UID: os.Getuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -108,14 +106,13 @@ func TestInitializeWithVolumes(t *testing.T) { } func TestCreate(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") rootDir, err := ioutil.TempDir("", "local-volume-test") if err != nil { t.Fatal(err) } defer os.RemoveAll(rootDir) - r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err := New(rootDir, idtools.IDPair{UID: os.Getuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -152,7 +149,7 @@ func TestCreate(t *testing.T) { } } - r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err = New(rootDir, idtools.IDPair{UID: os.Getuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -182,14 +179,14 @@ func TestValidateName(t *testing.T) { func TestCreateWithOpts(t *testing.T) { skip.If(t, runtime.GOOS == "windows") - skip.If(t, os.Getuid() != 0, "skipping test that requires root") + skip.If(t, os.Getuid() != 0, "requires mounts") rootDir, err := ioutil.TempDir("", "local-volume-test") if err != nil { t.Fatal(err) } defer os.RemoveAll(rootDir) - r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err := New(rootDir, idtools.IDPair{UID: os.Getuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -280,14 +277,13 @@ func TestCreateWithOpts(t *testing.T) { } func TestRelaodNoOpts(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") rootDir, err := ioutil.TempDir("", "volume-test-reload-no-opts") if err != nil { t.Fatal(err) } defer os.RemoveAll(rootDir) - r, err := New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err := New(rootDir, idtools.IDPair{UID: os.Getuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } @@ -315,7 +311,7 @@ func TestRelaodNoOpts(t *testing.T) { t.Fatal(err) } - r, err = New(rootDir, idtools.IDPair{UID: 0, GID: 0}) + r, err = New(rootDir, idtools.IDPair{UID: os.Getuid(), GID: os.Getegid()}) if err != nil { t.Fatal(err) } diff --git a/volume/store/store.go b/volume/store/store.go index 67b4e7d7ef..990bc3077b 100644 --- a/volume/store/store.go +++ b/volume/store/store.go @@ -80,7 +80,7 @@ func New(rootPath string, drivers *drivers.Store) (*VolumeStore, error) { if rootPath != "" { // initialize metadata store volPath := filepath.Join(rootPath, volumeDataDir) - if err := os.MkdirAll(volPath, 750); err != nil { + if err := os.MkdirAll(volPath, 0750); err != nil { return nil, err } diff --git a/volume/store/store_test.go b/volume/store/store_test.go index 255480a75f..288a4ce824 100644 --- a/volume/store/store_test.go +++ b/volume/store/store_test.go @@ -15,11 +15,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" - "github.com/gotestyourself/gotestyourself/skip" ) func TestCreate(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() s, cleanup := setupTest(t) @@ -49,7 +47,6 @@ func TestCreate(t *testing.T) { } func TestRemove(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() s, cleanup := setupTest(t) @@ -128,7 +125,6 @@ func TestList(t *testing.T) { } func TestFilterByDriver(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() s, cleanup := setupTest(t) defer cleanup() @@ -156,7 +152,6 @@ func TestFilterByDriver(t *testing.T) { } func TestFilterByUsed(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() s, cleanup := setupTest(t) defer cleanup() @@ -194,7 +189,6 @@ func TestFilterByUsed(t *testing.T) { } func TestDerefMultipleOfSameRef(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() s, cleanup := setupTest(t) defer cleanup() @@ -216,7 +210,6 @@ func TestDerefMultipleOfSameRef(t *testing.T) { } func TestCreateKeepOptsLabelsWhenExistsRemotely(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() s, cleanup := setupTest(t) defer cleanup() @@ -245,7 +238,6 @@ func TestCreateKeepOptsLabelsWhenExistsRemotely(t *testing.T) { } func TestDefererencePluginOnCreateError(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() var ( @@ -292,7 +284,6 @@ func TestDefererencePluginOnCreateError(t *testing.T) { } func TestRefDerefRemove(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() driverName := "test-ref-deref-remove" @@ -313,7 +304,6 @@ func TestRefDerefRemove(t *testing.T) { } func TestGet(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() driverName := "test-get" @@ -340,7 +330,6 @@ func TestGet(t *testing.T) { } func TestGetWithRef(t *testing.T) { - skip.If(t, os.Getuid() != 0, "skipping test that requires root") t.Parallel() driverName := "test-get-with-ref"