From 0322c3bf1f1094d1b6c23912892aa36c64ff7f08 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 --- registry/storage/driver/testsuites/testsuites.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/registry/storage/driver/testsuites/testsuites.go b/registry/storage/driver/testsuites/testsuites.go index f99df8d93..bb9d289d8 100644 --- a/registry/storage/driver/testsuites/testsuites.go +++ b/registry/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 a4eae0917e40bdc919c822b07d8231966ed9f1df 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 --- registry/storage/driver/filesystem/driver.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/registry/storage/driver/filesystem/driver.go b/registry/storage/driver/filesystem/driver.go index 7dece0b3f..480bd6873 100644 --- a/registry/storage/driver/filesystem/driver.go +++ b/registry/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 b45078eb442fe56b2df62f4089f045e3e16d7319 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 --- registry/storage/driver/storagedriver.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/registry/storage/driver/storagedriver.go b/registry/storage/driver/storagedriver.go index f15d50a9d..dc8bdc8d4 100644 --- a/registry/storage/driver/storagedriver.go +++ b/registry/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 a889f462235a8a997fccbdb96459a56d4299caff 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 --- registry/storage/driver/s3/s3.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/registry/storage/driver/s3/s3.go b/registry/storage/driver/s3/s3.go index 7672fbdbf..a9f303dc6 100644 --- a/registry/storage/driver/s3/s3.go +++ b/registry/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 f25ccea279407cbcdebdf50aae7fb3cfeb78b1bd 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 --- registry/storage/driver/rados/rados.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/storage/driver/rados/rados.go b/registry/storage/driver/rados/rados.go index 29bc32476..c2be528e6 100644 --- a/registry/storage/driver/rados/rados.go +++ b/registry/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 79d4d7f54655f9c209e5e6c613d8e7d7a078841f 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 --- registry/storage/driver/swift/swift.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/storage/driver/swift/swift.go b/registry/storage/driver/swift/swift.go index 6d021ea46..86bce794d 100644 --- a/registry/storage/driver/swift/swift.go +++ b/registry/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 7161fa055917e45f70e0f0ef727f60c70fe22fda 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 --- registry/storage/driver/oss/oss.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/registry/storage/driver/oss/oss.go b/registry/storage/driver/oss/oss.go index c16b9949a..e9e877a5d 100644 --- a/registry/storage/driver/oss/oss.go +++ b/registry/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 b6756a3d89d4f8e6e30941d0e95d6f7a3b469793 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 --- registry/storage/driver/oss/oss.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/registry/storage/driver/oss/oss.go b/registry/storage/driver/oss/oss.go index e9e877a5d..09b25ef0f 100644 --- a/registry/storage/driver/oss/oss.go +++ b/registry/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 66cd2bf95098f047cd52a9566a46062ef1922679 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 --- registry/storage/driver/oss/oss.go | 21 ++++++++++++--------- registry/storage/driver/s3/s3.go | 19 +++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/registry/storage/driver/oss/oss.go b/registry/storage/driver/oss/oss.go index 09b25ef0f..c6e4f8a32 100644 --- a/registry/storage/driver/oss/oss.go +++ b/registry/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/registry/storage/driver/s3/s3.go b/registry/storage/driver/s3/s3.go index a9f303dc6..7bb23a85d 100644 --- a/registry/storage/driver/s3/s3.go +++ b/registry/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 }