From 4829e9685ecdf72a99d26aa5326fbbc88603262d Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Fri, 13 Nov 2015 13:47:07 -0800 Subject: [PATCH 1/9] registry/storage/driver: checking that non-existent path returns PathNotFoundError Issue #1186 describes a condition where a null tags response is returned when using the s3 driver. The issue seems to be related to a missing PathNotFoundError in s3. This change adds a test for that to get an idea of the lack of compliance across storage drivers. If the failures are manageable, we'll add this test condition and fix the s3 driver. Signed-off-by: Stephen J Day --- docs/storage/driver/testsuites/testsuites.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/storage/driver/testsuites/testsuites.go b/docs/storage/driver/testsuites/testsuites.go index f99df8d9..bb9d289d 100644 --- a/docs/storage/driver/testsuites/testsuites.go +++ b/docs/storage/driver/testsuites/testsuites.go @@ -472,6 +472,13 @@ func (suite *DriverSuite) TestList(c *check.C) { rootDirectory := "/" + randomFilename(int64(8+rand.Intn(8))) defer suite.StorageDriver.Delete(suite.ctx, rootDirectory) + doesnotexist := path.Join(rootDirectory, "nonexistent") + _, err := suite.StorageDriver.List(suite.ctx, doesnotexist) + c.Assert(err, check.Equals, storagedriver.PathNotFoundError{ + Path: doesnotexist, + DriverName: suite.StorageDriver.Name(), + }) + parentDirectory := rootDirectory + "/" + randomFilename(int64(8+rand.Intn(8))) childFiles := make([]string, 50) for i := 0; i < len(childFiles); i++ { From c46d32bfbb68a5b6b331c0100c4e091e9e5da281 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 18 Nov 2015 16:11:44 +0100 Subject: [PATCH 2/9] driver/filesystem: address filesystem driver on behavior of List Signed-off-by: Stephen J Day --- docs/storage/driver/filesystem/driver.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/storage/driver/filesystem/driver.go b/docs/storage/driver/filesystem/driver.go index 7dece0b3..480bd687 100644 --- a/docs/storage/driver/filesystem/driver.go +++ b/docs/storage/driver/filesystem/driver.go @@ -184,9 +184,6 @@ func (d *driver) Stat(ctx context.Context, subPath string) (storagedriver.FileIn // List returns a list of the objects that are direct descendants of the given // path. func (d *driver) List(ctx context.Context, subPath string) ([]string, error) { - if subPath[len(subPath)-1] != '/' { - subPath += "/" - } fullPath := d.fullPath(subPath) dir, err := os.Open(fullPath) From dc5b71afb032cb2e7dbc5fde869abf4c5f901510 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 24 Nov 2015 14:17:25 -0800 Subject: [PATCH 3/9] storage/driver/base: use correct error format style Signed-off-by: Stephen J Day --- docs/storage/driver/storagedriver.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/storage/driver/storagedriver.go b/docs/storage/driver/storagedriver.go index f15d50a9..dc8bdc8d 100644 --- a/docs/storage/driver/storagedriver.go +++ b/docs/storage/driver/storagedriver.go @@ -97,7 +97,7 @@ type ErrUnsupportedMethod struct { } func (err ErrUnsupportedMethod) Error() string { - return fmt.Sprintf("[%s] unsupported method", err.DriverName) + return fmt.Sprintf("%s: unsupported method", err.DriverName) } // PathNotFoundError is returned when operating on a nonexistent path. @@ -107,7 +107,7 @@ type PathNotFoundError struct { } func (err PathNotFoundError) Error() string { - return fmt.Sprintf("[%s] Path not found: %s", err.DriverName, err.Path) + return fmt.Sprintf("%s: Path not found: %s", err.DriverName, err.Path) } // InvalidPathError is returned when the provided path is malformed. @@ -117,7 +117,7 @@ type InvalidPathError struct { } func (err InvalidPathError) Error() string { - return fmt.Sprintf("[%s] Invalid path: %s", err.DriverName, err.Path) + return fmt.Sprintf("%s: invalid path: %s", err.DriverName, err.Path) } // InvalidOffsetError is returned when attempting to read or write from an @@ -129,7 +129,7 @@ type InvalidOffsetError struct { } func (err InvalidOffsetError) Error() string { - return fmt.Sprintf("[%s] Invalid offset: %d for path: %s", err.DriverName, err.Offset, err.Path) + return fmt.Sprintf("%s: invalid offset: %d for path: %s", err.DriverName, err.Offset, err.Path) } // Error is a catch-all error type which captures an error string and @@ -140,5 +140,5 @@ type Error struct { } func (err Error) Error() string { - return fmt.Sprintf("[%s] %s", err.DriverName, err.Enclosed) + return fmt.Sprintf("%s: %s", err.DriverName, err.Enclosed) } From 10f7b7bf95f200630ab23ca1d2a01f48ef22d129 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 24 Nov 2015 14:23:12 -0800 Subject: [PATCH 4/9] storage/driver/s3: correct response on list of missing directory Signed-off-by: Stephen J Day --- docs/storage/driver/s3/s3.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/storage/driver/s3/s3.go b/docs/storage/driver/s3/s3.go index 7672fbdb..a9f303dc 100644 --- a/docs/storage/driver/s3/s3.go +++ b/docs/storage/driver/s3/s3.go @@ -685,6 +685,12 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { return nil, err } + if len(listResponse.Contents) == 0 { + // Treat empty response as missing directory, since we don't actually + // have directories in s3. + return nil, storagedriver.PathNotFoundError{Path: path} + } + files := []string{} directories := []string{} From c39158d48ca60f99d1274152b53d2f4fa7b4e5b8 Mon Sep 17 00:00:00 2001 From: Vincent Giersch Date: Wed, 25 Nov 2015 00:13:36 +0100 Subject: [PATCH 5/9] driver/rados: treat OMAP EIO as a PathNotFoundError RADOS returns a -EIO when trying to read a non-existing OMAP, treat it as a PathNotFoundError when trying to list a non existing virtual directory. Signed-off-by: Vincent Giersch --- docs/storage/driver/rados/rados.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/storage/driver/rados/rados.go b/docs/storage/driver/rados/rados.go index 29bc3247..c2be528e 100644 --- a/docs/storage/driver/rados/rados.go +++ b/docs/storage/driver/rados/rados.go @@ -404,7 +404,7 @@ func (d *driver) List(ctx context.Context, dirPath string) ([]string, error) { files, err := d.listDirectoryOid(dirPath) if err != nil { - return nil, err + return nil, storagedriver.PathNotFoundError{Path: dirPath} } keys := make([]string, 0, len(files)) From aa08ced9d73502222ad04931598ef7770e630d6d Mon Sep 17 00:00:00 2001 From: davidli Date: Tue, 1 Dec 2015 10:30:14 +0800 Subject: [PATCH 6/9] driver/swift: treat empty object list as a PathNotFoundError Swift returns an empty object list when trying to read a non-existing object path, treat it as a PathNotFoundError when trying to list a non existing virtual directory. Signed-off-by: David li --- docs/storage/driver/swift/swift.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/storage/driver/swift/swift.go b/docs/storage/driver/swift/swift.go index 6d021ea4..86bce794 100644 --- a/docs/storage/driver/swift/swift.go +++ b/docs/storage/driver/swift/swift.go @@ -589,7 +589,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { files = append(files, strings.TrimPrefix(strings.TrimSuffix(obj.Name, "/"), d.swiftPath("/"))) } - if err == swift.ContainerNotFound { + if err == swift.ContainerNotFound || (len(objects) == 0 && path != "/") { return files, storagedriver.PathNotFoundError{Path: path} } return files, err From 3a5c6446d851d25757ea84ab1f4b1a3ab5609c4b Mon Sep 17 00:00:00 2001 From: Li Yi Date: Tue, 1 Dec 2015 17:06:06 +0800 Subject: [PATCH 7/9] Fix for stevvooe:check-storage-drivers-list-path-not-found in OSS driver Change-Id: I5e96fe761d3833c962084fd2d597f47e8a72e7c2 Signed-off-by: Li Yi --- docs/storage/driver/oss/oss.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/storage/driver/oss/oss.go b/docs/storage/driver/oss/oss.go index c16b9949..e9e877a5 100644 --- a/docs/storage/driver/oss/oss.go +++ b/docs/storage/driver/oss/oss.go @@ -669,6 +669,12 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { return nil, err } + if len(listResponse.Contents) == 0 { + // Treat empty response as missing directory, since we don't actually + // have directories in OSS. + return nil, storagedriver.PathNotFoundError{Path: path} + } + files := []string{} directories := []string{} From 533c912d3ef08116102acde16d19bf42327b6064 Mon Sep 17 00:00:00 2001 From: Li Yi Date: Tue, 8 Dec 2015 19:55:28 +0800 Subject: [PATCH 8/9] Fix the issue for listing root directory Change-Id: I1c6181fa4e5666bd2e6ec69cb608c4778ae0fe48 Signed-off-by: Li Yi --- docs/storage/driver/oss/oss.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/storage/driver/oss/oss.go b/docs/storage/driver/oss/oss.go index e9e877a5..09b25ef0 100644 --- a/docs/storage/driver/oss/oss.go +++ b/docs/storage/driver/oss/oss.go @@ -666,10 +666,10 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { listResponse, err := d.Bucket.List(d.ossPath(path), "/", "", listMax) if err != nil { - return nil, err + return nil, parseError(path, err) } - if len(listResponse.Contents) == 0 { + if len(listResponse.Contents) == 0 && path != "/" { // Treat empty response as missing directory, since we don't actually // have directories in OSS. return nil, storagedriver.PathNotFoundError{Path: path} From d68acc869e89b7e54369c6bf13ff6b520783e927 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 8 Dec 2015 11:02:40 -0800 Subject: [PATCH 9/9] storage/driver/s3: adjust s3 driver to return unmunged path This fixes both the s3 driver and the oss driver to return the unmunged path when returning errors. Signed-off-by: Stephen J Day --- docs/storage/driver/oss/oss.go | 21 ++++++++++++--------- docs/storage/driver/s3/s3.go | 19 +++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/docs/storage/driver/oss/oss.go b/docs/storage/driver/oss/oss.go index 09b25ef0..c6e4f8a3 100644 --- a/docs/storage/driver/oss/oss.go +++ b/docs/storage/driver/oss/oss.go @@ -651,8 +651,9 @@ func (d *driver) Stat(ctx context.Context, path string) (storagedriver.FileInfo, } // List returns a list of the objects that are direct descendants of the given path. -func (d *driver) List(ctx context.Context, path string) ([]string, error) { - if path != "/" && path[len(path)-1] != '/' { +func (d *driver) List(ctx context.Context, opath string) ([]string, error) { + path := opath + if path != "/" && opath[len(path)-1] != '/' { path = path + "/" } @@ -666,13 +667,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { listResponse, err := d.Bucket.List(d.ossPath(path), "/", "", listMax) if err != nil { - return nil, parseError(path, err) - } - - if len(listResponse.Contents) == 0 && path != "/" { - // Treat empty response as missing directory, since we don't actually - // have directories in OSS. - return nil, storagedriver.PathNotFoundError{Path: path} + return nil, parseError(opath, err) } files := []string{} @@ -697,6 +692,14 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { } } + if opath != "/" { + if len(files) == 0 && len(directories) == 0 { + // Treat empty response as missing directory, since we don't actually + // have directories in s3. + return nil, storagedriver.PathNotFoundError{Path: opath} + } + } + return append(files, directories...), nil } diff --git a/docs/storage/driver/s3/s3.go b/docs/storage/driver/s3/s3.go index a9f303dc..7bb23a85 100644 --- a/docs/storage/driver/s3/s3.go +++ b/docs/storage/driver/s3/s3.go @@ -667,7 +667,8 @@ func (d *driver) Stat(ctx context.Context, path string) (storagedriver.FileInfo, } // List returns a list of the objects that are direct descendants of the given path. -func (d *driver) List(ctx context.Context, path string) ([]string, error) { +func (d *driver) List(ctx context.Context, opath string) ([]string, error) { + path := opath if path != "/" && path[len(path)-1] != '/' { path = path + "/" } @@ -682,13 +683,7 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { listResponse, err := d.Bucket.List(d.s3Path(path), "/", "", listMax) if err != nil { - return nil, err - } - - if len(listResponse.Contents) == 0 { - // Treat empty response as missing directory, since we don't actually - // have directories in s3. - return nil, storagedriver.PathNotFoundError{Path: path} + return nil, parseError(opath, err) } files := []string{} @@ -713,6 +708,14 @@ func (d *driver) List(ctx context.Context, path string) ([]string, error) { } } + if opath != "/" { + if len(files) == 0 && len(directories) == 0 { + // Treat empty response as missing directory, since we don't actually + // have directories in s3. + return nil, storagedriver.PathNotFoundError{Path: opath} + } + } + return append(files, directories...), nil }