diff --git a/registry/storage/blobserver.go b/registry/storage/blobserver.go index 24aeba69..45f81f53 100644 --- a/registry/storage/blobserver.go +++ b/registry/storage/blobserver.go @@ -36,7 +36,7 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h redirectURL, err := bs.driver.URLFor(ctx, path, map[string]interface{}{"method": r.Method}) - switch err { + switch err.(type) { case nil: if bs.redirect { // Redirect to storage URL. @@ -44,7 +44,6 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h return err } - fallthrough case driver.ErrUnsupportedMethod: // Fallback to serving the content directly. br, err := newFileReader(ctx, bs.driver, path, desc.Size) diff --git a/registry/storage/driver/base/base.go b/registry/storage/driver/base/base.go index 60af06b8..c816d2d6 100644 --- a/registry/storage/driver/base/base.go +++ b/registry/storage/driver/base/base.go @@ -50,16 +50,44 @@ type Base struct { storagedriver.StorageDriver } +// Format errors received from the storage driver +func (base *Base) setDriverName(e error) error { + switch actual := e.(type) { + case nil: + return nil + case storagedriver.ErrUnsupportedMethod: + actual.DriverName = base.StorageDriver.Name() + return actual + case storagedriver.PathNotFoundError: + actual.DriverName = base.StorageDriver.Name() + return actual + case storagedriver.InvalidPathError: + actual.DriverName = base.StorageDriver.Name() + return actual + case storagedriver.InvalidOffsetError: + actual.DriverName = base.StorageDriver.Name() + return actual + default: + storageError := storagedriver.Error{ + DriverName: base.StorageDriver.Name(), + Enclosed: e, + } + + return storageError + } +} + // GetContent wraps GetContent of underlying storage driver. func (base *Base) GetContent(ctx context.Context, path string) ([]byte, error) { ctx, done := context.WithTrace(ctx) defer done("%s.GetContent(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.GetContent(ctx, path) + b, e := base.StorageDriver.GetContent(ctx, path) + return b, base.setDriverName(e) } // PutContent wraps PutContent of underlying storage driver. @@ -68,10 +96,10 @@ func (base *Base) PutContent(ctx context.Context, path string, content []byte) e defer done("%s.PutContent(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return storagedriver.InvalidPathError{Path: path} + return storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.PutContent(ctx, path, content) + return base.setDriverName(base.StorageDriver.PutContent(ctx, path, content)) } // ReadStream wraps ReadStream of underlying storage driver. @@ -80,14 +108,15 @@ func (base *Base) ReadStream(ctx context.Context, path string, offset int64) (io defer done("%s.ReadStream(%q, %d)", base.Name(), path, offset) if offset < 0 { - return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset} + return nil, storagedriver.InvalidOffsetError{Path: path, Offset: offset, DriverName: base.StorageDriver.Name()} } if !storagedriver.PathRegexp.MatchString(path) { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.ReadStream(ctx, path, offset) + rc, e := base.StorageDriver.ReadStream(ctx, path, offset) + return rc, base.setDriverName(e) } // WriteStream wraps WriteStream of underlying storage driver. @@ -96,14 +125,15 @@ func (base *Base) WriteStream(ctx context.Context, path string, offset int64, re defer done("%s.WriteStream(%q, %d)", base.Name(), path, offset) if offset < 0 { - return 0, storagedriver.InvalidOffsetError{Path: path, Offset: offset} + return 0, storagedriver.InvalidOffsetError{Path: path, Offset: offset, DriverName: base.StorageDriver.Name()} } if !storagedriver.PathRegexp.MatchString(path) { - return 0, storagedriver.InvalidPathError{Path: path} + return 0, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.WriteStream(ctx, path, offset, reader) + i64, e := base.StorageDriver.WriteStream(ctx, path, offset, reader) + return i64, base.setDriverName(e) } // Stat wraps Stat of underlying storage driver. @@ -112,10 +142,11 @@ func (base *Base) Stat(ctx context.Context, path string) (storagedriver.FileInfo defer done("%s.Stat(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.Stat(ctx, path) + fi, e := base.StorageDriver.Stat(ctx, path) + return fi, base.setDriverName(e) } // List wraps List of underlying storage driver. @@ -124,10 +155,11 @@ func (base *Base) List(ctx context.Context, path string) ([]string, error) { defer done("%s.List(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) && path != "/" { - return nil, storagedriver.InvalidPathError{Path: path} + return nil, storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.List(ctx, path) + str, e := base.StorageDriver.List(ctx, path) + return str, base.setDriverName(e) } // Move wraps Move of underlying storage driver. @@ -136,12 +168,12 @@ func (base *Base) Move(ctx context.Context, sourcePath string, destPath string) defer done("%s.Move(%q, %q", base.Name(), sourcePath, destPath) if !storagedriver.PathRegexp.MatchString(sourcePath) { - return storagedriver.InvalidPathError{Path: sourcePath} + return storagedriver.InvalidPathError{Path: sourcePath, DriverName: base.StorageDriver.Name()} } else if !storagedriver.PathRegexp.MatchString(destPath) { - return storagedriver.InvalidPathError{Path: destPath} + return storagedriver.InvalidPathError{Path: destPath, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.Move(ctx, sourcePath, destPath) + return base.setDriverName(base.StorageDriver.Move(ctx, sourcePath, destPath)) } // Delete wraps Delete of underlying storage driver. @@ -150,10 +182,10 @@ func (base *Base) Delete(ctx context.Context, path string) error { defer done("%s.Delete(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return storagedriver.InvalidPathError{Path: path} + return storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.Delete(ctx, path) + return base.setDriverName(base.StorageDriver.Delete(ctx, path)) } // URLFor wraps URLFor of underlying storage driver. @@ -162,8 +194,9 @@ func (base *Base) URLFor(ctx context.Context, path string, options map[string]in defer done("%s.URLFor(%q)", base.Name(), path) if !storagedriver.PathRegexp.MatchString(path) { - return "", storagedriver.InvalidPathError{Path: path} + return "", storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } - return base.StorageDriver.URLFor(ctx, path, options) + str, e := base.StorageDriver.URLFor(ctx, path, options) + return str, base.setDriverName(e) } diff --git a/registry/storage/driver/filesystem/driver.go b/registry/storage/driver/filesystem/driver.go index d5d8708c..7dece0b3 100644 --- a/registry/storage/driver/filesystem/driver.go +++ b/registry/storage/driver/filesystem/driver.go @@ -248,7 +248,7 @@ func (d *driver) Delete(ctx context.Context, subPath string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } // fullPath returns the absolute path of a key within the Driver's storage. diff --git a/registry/storage/driver/gcs/gcs.go b/registry/storage/driver/gcs/gcs.go index 8dc96675..4cef972c 100644 --- a/registry/storage/driver/gcs/gcs.go +++ b/registry/storage/driver/gcs/gcs.go @@ -575,7 +575,7 @@ func (d *driver) Delete(context ctx.Context, path string) error { // Returns ErrUnsupportedMethod if this driver has no privateKey func (d *driver) URLFor(context ctx.Context, path string, options map[string]interface{}) (string, error) { if d.privateKey == nil { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } name := d.pathToKey(path) @@ -584,7 +584,7 @@ func (d *driver) URLFor(context ctx.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } } diff --git a/registry/storage/driver/inmemory/driver.go b/registry/storage/driver/inmemory/driver.go index 2d121e1c..b5735c0a 100644 --- a/registry/storage/driver/inmemory/driver.go +++ b/registry/storage/driver/inmemory/driver.go @@ -258,5 +258,5 @@ func (d *driver) Delete(ctx context.Context, path string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } diff --git a/registry/storage/driver/oss/oss.go b/registry/storage/driver/oss/oss.go index cec32026..c16b9949 100644 --- a/registry/storage/driver/oss/oss.go +++ b/registry/storage/driver/oss/oss.go @@ -748,7 +748,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } } diff --git a/registry/storage/driver/rados/rados.go b/registry/storage/driver/rados/rados.go index b2e6590d..29bc3247 100644 --- a/registry/storage/driver/rados/rados.go +++ b/registry/storage/driver/rados/rados.go @@ -496,7 +496,7 @@ func (d *driver) Delete(ctx context.Context, objectPath string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. // May return an UnsupportedMethodErr in certain StorageDriver implementations. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } // Generate a blob identifier diff --git a/registry/storage/driver/s3/s3.go b/registry/storage/driver/s3/s3.go index 46dbcd7f..7672fbdb 100644 --- a/registry/storage/driver/s3/s3.go +++ b/registry/storage/driver/s3/s3.go @@ -759,7 +759,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int if ok { methodString, ok = method.(string) if !ok || (methodString != "GET" && methodString != "HEAD") { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } } diff --git a/registry/storage/driver/storagedriver.go b/registry/storage/driver/storagedriver.go index bade099f..cd1c883b 100644 --- a/registry/storage/driver/storagedriver.go +++ b/registry/storage/driver/storagedriver.go @@ -1,7 +1,6 @@ package driver import ( - "errors" "fmt" "io" "regexp" @@ -93,33 +92,53 @@ type StorageDriver interface { var PathRegexp = regexp.MustCompile(`^(/[A-Za-z0-9._-]+)+$`) // ErrUnsupportedMethod may be returned in the case where a StorageDriver implementation does not support an optional method. -var ErrUnsupportedMethod = errors.New("unsupported method") +type ErrUnsupportedMethod struct { + DriverName string +} + +func (err ErrUnsupportedMethod) Error() string { + return fmt.Sprintf("[%s] unsupported method", err.DriverName) +} // PathNotFoundError is returned when operating on a nonexistent path. type PathNotFoundError struct { - Path string + Path string + DriverName string } func (err PathNotFoundError) Error() string { - return fmt.Sprintf("Path not found: %s", err.Path) + return fmt.Sprintf("[%s] Path not found: %s", err.DriverName, err.Path) } // InvalidPathError is returned when the provided path is malformed. type InvalidPathError struct { - Path string + Path string + DriverName string } func (err InvalidPathError) Error() string { - return fmt.Sprintf("Invalid path: %s", err.Path) + return fmt.Sprintf("[%s] Invalid path: %s", err.DriverName, err.Path) } // InvalidOffsetError is returned when attempting to read or write from an // invalid offset. type InvalidOffsetError struct { - Path string - Offset int64 + Path string + Offset int64 + DriverName string } func (err InvalidOffsetError) Error() string { - return fmt.Sprintf("Invalid offset: %d for path: %s", 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 +// the driver type on which it occured. +type Error struct { + DriverName string + Enclosed error +} + +func (err Error) Error() string { + return fmt.Sprintf("[%s] %s", err.DriverName, err.Enclosed) } diff --git a/registry/storage/driver/swift/swift.go b/registry/storage/driver/swift/swift.go index 3b2cdc53..bd330925 100644 --- a/registry/storage/driver/swift/swift.go +++ b/registry/storage/driver/swift/swift.go @@ -658,14 +658,14 @@ func (d *driver) Delete(ctx context.Context, path string) error { // URLFor returns a URL which may be used to retrieve the content stored at the given path. func (d *driver) URLFor(ctx context.Context, path string, options map[string]interface{}) (string, error) { if d.SecretKey == "" { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } methodString := "GET" method, ok := options["method"] if ok { if methodString, ok = method.(string); !ok { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } } @@ -684,7 +684,7 @@ func (d *driver) URLFor(ctx context.Context, path string, options map[string]int } if !supported { - return "", storagedriver.ErrUnsupportedMethod + return "", storagedriver.ErrUnsupportedMethod{} } expiresTime := time.Now().Add(20 * time.Minute) diff --git a/registry/storage/driver/testsuites/testsuites.go b/registry/storage/driver/testsuites/testsuites.go index 1772560b..f99df8d9 100644 --- a/registry/storage/driver/testsuites/testsuites.go +++ b/registry/storage/driver/testsuites/testsuites.go @@ -10,6 +10,7 @@ import ( "os" "path" "sort" + "strings" "sync" "testing" "time" @@ -145,10 +146,12 @@ func (suite *DriverSuite) TestInvalidPaths(c *check.C) { defer suite.StorageDriver.Delete(suite.ctx, firstPart(filename)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.InvalidPathError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.InvalidPathError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } } @@ -205,6 +208,7 @@ func (suite *DriverSuite) TestReadNonexistent(c *check.C) { _, err := suite.StorageDriver.GetContent(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestWriteReadStreams1 tests a simple write-read streaming workflow. @@ -321,6 +325,7 @@ func (suite *DriverSuite) TestReadStreamWithOffset(c *check.C) { c.Assert(err.(storagedriver.InvalidOffsetError).Offset, check.Equals, int64(-1)) c.Assert(err.(storagedriver.InvalidOffsetError).Path, check.Equals, filename) c.Assert(reader, check.IsNil) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) // Read past the end of the content and make sure we get a reader that // returns 0 bytes and io.EOF @@ -443,6 +448,7 @@ func (suite *DriverSuite) testContinueStreamAppend(c *check.C, chunkSize int64) c.Assert(err, check.FitsTypeOf, storagedriver.InvalidOffsetError{}) c.Assert(err.(storagedriver.InvalidOffsetError).Path, check.Equals, filename) c.Assert(err.(storagedriver.InvalidOffsetError).Offset, check.Equals, int64(-1)) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestReadNonexistentStream tests that reading a stream for a nonexistent path @@ -453,10 +459,12 @@ func (suite *DriverSuite) TestReadNonexistentStream(c *check.C) { _, err := suite.StorageDriver.ReadStream(suite.ctx, filename, 0) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.ReadStream(suite.ctx, filename, 64) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestList checks the returned list of keys after populating a directory tree. @@ -517,6 +525,7 @@ func (suite *DriverSuite) TestMove(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, sourcePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestMoveOverwrite checks that a moved object no longer exists at the source @@ -546,6 +555,7 @@ func (suite *DriverSuite) TestMoveOverwrite(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, sourcePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestMoveNonexistent checks that moving a nonexistent key fails and does not @@ -563,6 +573,7 @@ func (suite *DriverSuite) TestMoveNonexistent(c *check.C) { err = suite.StorageDriver.Move(suite.ctx, sourcePath, destPath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) received, err := suite.StorageDriver.GetContent(suite.ctx, destPath) c.Assert(err, check.IsNil) @@ -600,6 +611,7 @@ func (suite *DriverSuite) TestDelete(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestURLFor checks that the URLFor method functions properly, but only if it @@ -614,7 +626,7 @@ func (suite *DriverSuite) TestURLFor(c *check.C) { c.Assert(err, check.IsNil) url, err := suite.StorageDriver.URLFor(suite.ctx, filename, nil) - if err == storagedriver.ErrUnsupportedMethod { + if _, ok := err.(storagedriver.ErrUnsupportedMethod); ok { return } c.Assert(err, check.IsNil) @@ -628,7 +640,7 @@ func (suite *DriverSuite) TestURLFor(c *check.C) { c.Assert(read, check.DeepEquals, contents) url, err = suite.StorageDriver.URLFor(suite.ctx, filename, map[string]interface{}{"method": "HEAD"}) - if err == storagedriver.ErrUnsupportedMethod { + if _, ok := err.(storagedriver.ErrUnsupportedMethod); ok { return } c.Assert(err, check.IsNil) @@ -644,6 +656,7 @@ func (suite *DriverSuite) TestDeleteNonexistent(c *check.C) { err := suite.StorageDriver.Delete(suite.ctx, filename) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestDeleteFolder checks that deleting a folder removes all child elements. @@ -671,6 +684,7 @@ func (suite *DriverSuite) TestDeleteFolder(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename1)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename2)) c.Assert(err, check.IsNil) @@ -684,14 +698,17 @@ func (suite *DriverSuite) TestDeleteFolder(c *check.C) { _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename1)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename2)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) _, err = suite.StorageDriver.GetContent(suite.ctx, path.Join(dirname, filename3)) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) } // TestStatCall runs verifies the implementation of the storagedriver's Stat call. @@ -707,11 +724,13 @@ func (suite *DriverSuite) TestStatCall(c *check.C) { fi, err := suite.StorageDriver.Stat(suite.ctx, dirPath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) c.Assert(fi, check.IsNil) fi, err = suite.StorageDriver.Stat(suite.ctx, filePath) c.Assert(err, check.NotNil) c.Assert(err, check.FitsTypeOf, storagedriver.PathNotFoundError{}) + c.Assert(strings.Contains(err.Error(), suite.Name()), check.Equals, true) c.Assert(fi, check.IsNil) err = suite.StorageDriver.PutContent(suite.ctx, filePath, content)