From 0a6a6f5b81540bc3cc4e87eeba098d884884dded Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Thu, 14 May 2015 18:21:39 -0700 Subject: [PATCH 1/4] Move ErrorCode logic to new errcode package Make HTTP status codes match the ErrorCode by looking it up in the Descriptors Signed-off-by: Doug Davis --- registry/api/errcode/errors.go | 206 +++++++++++++++ registry/api/{v2 => errcode}/errors_test.go | 8 +- registry/api/v2/descriptors.go | 223 +++------------- registry/api/v2/errors.go | 270 ++++++++++---------- registry/client/blob_writer_test.go | 5 +- registry/client/errors.go | 5 +- registry/client/repository_test.go | 3 +- registry/handlers/api_test.go | 11 +- registry/handlers/app.go | 30 ++- registry/handlers/app_test.go | 11 +- registry/handlers/blob.go | 7 +- registry/handlers/blobupload.go | 45 +--- registry/handlers/context.go | 3 +- registry/handlers/helpers.go | 12 + registry/handlers/images.go | 9 - registry/handlers/tags.go | 1 - 16 files changed, 444 insertions(+), 405 deletions(-) create mode 100644 registry/api/errcode/errors.go rename registry/api/{v2 => errcode}/errors_test.go (98%) diff --git a/registry/api/errcode/errors.go b/registry/api/errcode/errors.go new file mode 100644 index 000000000..ce3c06246 --- /dev/null +++ b/registry/api/errcode/errors.go @@ -0,0 +1,206 @@ +package errcode + +import ( + "fmt" + "net/http" + "strings" +) + +// ErrorCode represents the error type. The errors are serialized via strings +// and the integer format may change and should *never* be exported. +type ErrorCode int + +// ErrorDescriptor provides relevant information about a given error code. +type ErrorDescriptor struct { + // Code is the error code that this descriptor describes. + Code ErrorCode + + // Value provides a unique, string key, often captilized with + // underscores, to identify the error code. This value is used as the + // keyed value when serializing api errors. + Value string + + // Message is a short, human readable decription of the error condition + // included in API responses. + Message string + + // Description provides a complete account of the errors purpose, suitable + // for use in documentation. + Description string + + // HTTPStatusCode provides the http status code that is associated with + // this error condition. + HTTPStatusCode int +} + +var ( + errorCodeToDescriptors = map[ErrorCode]ErrorDescriptor{} + idToDescriptors = map[string]ErrorDescriptor{} +) + +const ( + // ErrorCodeUnknown is a catch-all for errors not defined below. + ErrorCodeUnknown ErrorCode = 10000 + iota +) + +var errorDescriptors = []ErrorDescriptor{ + { + Code: ErrorCodeUnknown, + Value: "UNKNOWN", + Message: "unknown error", + Description: `Generic error returned when the error does not have an + API classification.`, + HTTPStatusCode: http.StatusInternalServerError, + }, +} + +// LoadErrors will register a new set of Errors into the system +func LoadErrors(errs *[]ErrorDescriptor) { + for _, descriptor := range *errs { + if _, ok := idToDescriptors[descriptor.Value]; ok { + panic(fmt.Sprintf("ErrorValue %s is already registered", descriptor.Value)) + } + if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { + panic(fmt.Sprintf("ErrorCode %d is already registered", descriptor.Code)) + } + + errorCodeToDescriptors[descriptor.Code] = descriptor + idToDescriptors[descriptor.Value] = descriptor + } +} + +// ParseErrorCode attempts to parse the error code string, returning +// ErrorCodeUnknown if the error is not known. +func ParseErrorCode(s string) ErrorCode { + desc, ok := idToDescriptors[s] + + if !ok { + return ErrorCodeUnknown + } + + return desc.Code +} + +// Descriptor returns the descriptor for the error code. +func (ec ErrorCode) Descriptor() ErrorDescriptor { + d, ok := errorCodeToDescriptors[ec] + + if !ok { + return ErrorCodeUnknown.Descriptor() + } + + return d +} + +// String returns the canonical identifier for this error code. +func (ec ErrorCode) String() string { + return ec.Descriptor().Value +} + +// Message returned the human-readable error message for this error code. +func (ec ErrorCode) Message() string { + return ec.Descriptor().Message +} + +// MarshalText encodes the receiver into UTF-8-encoded text and returns the +// result. +func (ec ErrorCode) MarshalText() (text []byte, err error) { + return []byte(ec.String()), nil +} + +// UnmarshalText decodes the form generated by MarshalText. +func (ec *ErrorCode) UnmarshalText(text []byte) error { + desc, ok := idToDescriptors[string(text)] + + if !ok { + desc = ErrorCodeUnknown.Descriptor() + } + + *ec = desc.Code + + return nil +} + +// Error provides a wrapper around ErrorCode with extra Details provided. +type Error struct { + Code ErrorCode `json:"code"` + Message string `json:"message,omitempty"` + Detail interface{} `json:"detail,omitempty"` +} + +// Error returns a human readable representation of the error. +func (e Error) Error() string { + return fmt.Sprintf("%s: %s", + strings.ToLower(strings.Replace(e.Code.String(), "_", " ", -1)), + e.Message) +} + +// Errors provides the envelope for multiple errors and a few sugar methods +// for use within the application. +type Errors struct { + Errors []Error `json:"errors,omitempty"` +} + +// Push pushes an error on to the error stack, with the optional detail +// argument. It is a programming error (ie panic) to push more than one +// detail at a time. +func (errs *Errors) Push(code ErrorCode, details ...interface{}) { + if len(details) > 1 { + panic("please specify zero or one detail items for this error") + } + + var detail interface{} + if len(details) > 0 { + detail = details[0] + } + + if err, ok := detail.(error); ok { + detail = err.Error() + } + + errs.PushErr(Error{ + Code: code, + Message: code.Message(), + Detail: detail, + }) +} + +// PushErr pushes an error interface onto the error stack. +func (errs *Errors) PushErr(err error) { + switch err.(type) { + case Error: + errs.Errors = append(errs.Errors, err.(Error)) + default: + errs.Errors = append(errs.Errors, Error{Message: err.Error()}) + } +} + +func (errs *Errors) Error() string { + switch errs.Len() { + case 0: + return "" + case 1: + return errs.Errors[0].Error() + default: + msg := "errors:\n" + for _, err := range errs.Errors { + msg += err.Error() + "\n" + } + return msg + } +} + +// Clear clears the errors. +func (errs *Errors) Clear() { + errs.Errors = nil +} + +// Len returns the current number of errors. +func (errs *Errors) Len() int { + return len(errs.Errors) +} + +// init loads the default errors that are part of the errcode package +func init() { + LoadErrors(&errorDescriptors) +} diff --git a/registry/api/v2/errors_test.go b/registry/api/errcode/errors_test.go similarity index 98% rename from registry/api/v2/errors_test.go rename to registry/api/errcode/errors_test.go index 9cc831c44..eedb22ed4 100644 --- a/registry/api/v2/errors_test.go +++ b/registry/api/errcode/errors_test.go @@ -1,11 +1,11 @@ -package v2 +package errcode import ( "encoding/json" - "reflect" + // "reflect" "testing" - "github.com/docker/distribution/digest" + // "github.com/docker/distribution/digest" ) // TestErrorCodes ensures that error code format, mappings and @@ -56,6 +56,7 @@ func TestErrorCodes(t *testing.T) { // TestErrorsManagement does a quick check of the Errors type to ensure that // members are properly pushed and marshaled. +/* func TestErrorsManagement(t *testing.T) { var errs Errors @@ -163,3 +164,4 @@ func TestMarshalUnmarshal(t *testing.T) { t.Fatalf("errors not equal after round trip: %#v != %#v", unmarshaled, errors) } } +*/ diff --git a/registry/api/v2/descriptors.go b/registry/api/v2/descriptors.go index d7c4a880c..db5a92707 100644 --- a/registry/api/v2/descriptors.go +++ b/registry/api/v2/descriptors.go @@ -5,6 +5,7 @@ import ( "regexp" "github.com/docker/distribution/digest" + "github.com/docker/distribution/registry/api/errcode" ) var ( @@ -98,7 +99,7 @@ var ( Format: "", }, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, Body: BodyDescriptor{ @@ -119,7 +120,7 @@ var ( Format: "", }, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, Body: BodyDescriptor{ @@ -174,7 +175,7 @@ var APIDescriptor = struct { // ErrorDescriptors provides a list of the error codes and their // associated documentation and metadata. - ErrorDescriptors []ErrorDescriptor + ErrorDescriptors []errcode.ErrorDescriptor }{ RouteDescriptors: routeDescriptors, ErrorDescriptors: errorDescriptors, @@ -275,7 +276,7 @@ type ResponseDescriptor struct { // ErrorCodes enumerates the error codes that may be returned along with // the response. - ErrorCodes []ErrorCode + ErrorCodes []errcode.ErrorCode // Body describes the body of the response, if any. Body BodyDescriptor @@ -317,30 +318,6 @@ type ParameterDescriptor struct { Examples []string } -// ErrorDescriptor provides relevant information about a given error code. -type ErrorDescriptor struct { - // Code is the error code that this descriptor describes. - Code ErrorCode - - // Value provides a unique, string key, often captilized with - // underscores, to identify the error code. This value is used as the - // keyed value when serializing api errors. - Value string - - // Message is a short, human readable decription of the error condition - // included in API responses. - Message string - - // Description provides a complete account of the errors purpose, suitable - // for use in documentation. - Description string - - // HTTPStatusCodes provides a list of status under which this error - // condition may arise. If it is empty, the error condition may be seen - // for any status code. - HTTPStatusCodes []int -} - var routeDescriptors = []RouteDescriptor{ { Name: RouteNameBase, @@ -374,7 +351,7 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, }, @@ -438,7 +415,7 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameUnknown, }, }, @@ -449,7 +426,7 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, }, @@ -495,7 +472,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "The name or reference was invalid.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameInvalid, ErrorCodeTagInvalid, }, @@ -511,14 +488,14 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, }, { Description: "The named manifest is not known to the registry.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameUnknown, ErrorCodeManifestUnknown, }, @@ -573,7 +550,7 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameInvalid, ErrorCodeTagInvalid, ErrorCodeManifestInvalid, @@ -588,7 +565,7 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, }, @@ -596,7 +573,7 @@ var routeDescriptors = []RouteDescriptor{ Name: "Missing Layer(s)", Description: "One or more layers may be missing during a manifest upload. If so, the missing layers will be enumerated in the error response.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeBlobUnknown, }, Body: BodyDescriptor{ @@ -625,7 +602,7 @@ var routeDescriptors = []RouteDescriptor{ Format: "", }, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, Body: BodyDescriptor{ @@ -660,7 +637,7 @@ var routeDescriptors = []RouteDescriptor{ Name: "Invalid Name or Tag", Description: "The specified `name` or `tag` were invalid and the delete was unable to proceed.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameInvalid, ErrorCodeTagInvalid, }, @@ -680,7 +657,7 @@ var routeDescriptors = []RouteDescriptor{ Format: "", }, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeUnauthorized, }, Body: BodyDescriptor{ @@ -692,7 +669,7 @@ var routeDescriptors = []RouteDescriptor{ Name: "Unknown Manifest", Description: "The specified `name` or `tag` are unknown to the registry and the delete was unable to proceed. Clients can assume the manifest was already deleted if this response is returned.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameUnknown, ErrorCodeManifestUnknown, }, @@ -765,7 +742,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "There was a problem with the request that needs to be addressed by the client, such as an invalid `name` or `tag`.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameInvalid, ErrorCodeDigestInvalid, }, @@ -782,7 +759,7 @@ var routeDescriptors = []RouteDescriptor{ ContentType: "application/json; charset=utf-8", Format: errorsBody, }, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameUnknown, ErrorCodeBlobUnknown, }, @@ -834,7 +811,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "There was a problem with the request that needs to be addressed by the client, such as an invalid `name` or `tag`.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameInvalid, ErrorCodeDigestInvalid, }, @@ -846,7 +823,7 @@ var routeDescriptors = []RouteDescriptor{ unauthorizedResponse, { StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameUnknown, ErrorCodeBlobUnknown, }, @@ -926,7 +903,7 @@ var routeDescriptors = []RouteDescriptor{ { Name: "Invalid Name or Digest", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeDigestInvalid, ErrorCodeNameInvalid, }, @@ -970,7 +947,7 @@ var routeDescriptors = []RouteDescriptor{ { Name: "Invalid Name or Digest", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeDigestInvalid, ErrorCodeNameInvalid, }, @@ -1024,7 +1001,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "There was an error processing the upload and it must be restarted.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeDigestInvalid, ErrorCodeNameInvalid, ErrorCodeBlobUploadInvalid, @@ -1038,7 +1015,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "The upload is unknown to the registry. The upload must be restarted.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeBlobUploadUnknown, }, Body: BodyDescriptor{ @@ -1096,7 +1073,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "There was an error processing the upload and it must be restarted.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeDigestInvalid, ErrorCodeNameInvalid, ErrorCodeBlobUploadInvalid, @@ -1110,7 +1087,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "The upload is unknown to the registry. The upload must be restarted.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeBlobUploadUnknown, }, Body: BodyDescriptor{ @@ -1175,7 +1152,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "There was an error processing the upload and it must be restarted.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeDigestInvalid, ErrorCodeNameInvalid, ErrorCodeBlobUploadInvalid, @@ -1189,7 +1166,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "The upload is unknown to the registry. The upload must be restarted.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeBlobUploadUnknown, }, Body: BodyDescriptor{ @@ -1266,7 +1243,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "There was an error processing the upload and it must be restarted.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeDigestInvalid, ErrorCodeNameInvalid, ErrorCodeBlobUploadInvalid, @@ -1280,7 +1257,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "The upload is unknown to the registry. The upload must be restarted.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeBlobUploadUnknown, }, Body: BodyDescriptor{ @@ -1321,7 +1298,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "An error was encountered processing the delete. The client may ignore this error.", StatusCode: http.StatusBadRequest, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeNameInvalid, ErrorCodeBlobUploadInvalid, }, @@ -1334,7 +1311,7 @@ var routeDescriptors = []RouteDescriptor{ { Description: "The upload is unknown to the registry. The client may ignore this error and assume the upload has been deleted.", StatusCode: http.StatusNotFound, - ErrorCodes: []ErrorCode{ + ErrorCodes: []errcode.ErrorCode{ ErrorCodeBlobUploadUnknown, }, Body: BodyDescriptor{ @@ -1350,143 +1327,11 @@ var routeDescriptors = []RouteDescriptor{ }, } -// ErrorDescriptors provides a list of HTTP API Error codes that may be -// encountered when interacting with the registry API. -var errorDescriptors = []ErrorDescriptor{ - { - Code: ErrorCodeUnknown, - Value: "UNKNOWN", - Message: "unknown error", - Description: `Generic error returned when the error does not have an - API classification.`, - }, - { - Code: ErrorCodeUnsupported, - Value: "UNSUPPORTED", - Message: "The operation is unsupported.", - Description: `The operation was unsupported due to a missing - implementation or invalid set of parameters.`, - }, - { - Code: ErrorCodeUnauthorized, - Value: "UNAUTHORIZED", - Message: "access to the requested resource is not authorized", - Description: `The access controller denied access for the operation on - a resource. Often this will be accompanied by a 401 Unauthorized - response status.`, - }, - { - Code: ErrorCodeDigestInvalid, - Value: "DIGEST_INVALID", - Message: "provided digest did not match uploaded content", - Description: `When a blob is uploaded, the registry will check that - the content matches the digest provided by the client. The error may - include a detail structure with the key "digest", including the - invalid digest string. This error may also be returned when a manifest - includes an invalid layer digest.`, - HTTPStatusCodes: []int{http.StatusBadRequest, http.StatusNotFound}, - }, - { - Code: ErrorCodeSizeInvalid, - Value: "SIZE_INVALID", - Message: "provided length did not match content length", - Description: `When a layer is uploaded, the provided size will be - checked against the uploaded content. If they do not match, this error - will be returned.`, - HTTPStatusCodes: []int{http.StatusBadRequest}, - }, - { - Code: ErrorCodeNameInvalid, - Value: "NAME_INVALID", - Message: "invalid repository name", - Description: `Invalid repository name encountered either during - manifest validation or any API operation.`, - HTTPStatusCodes: []int{http.StatusBadRequest, http.StatusNotFound}, - }, - { - Code: ErrorCodeTagInvalid, - Value: "TAG_INVALID", - Message: "manifest tag did not match URI", - Description: `During a manifest upload, if the tag in the manifest - does not match the uri tag, this error will be returned.`, - HTTPStatusCodes: []int{http.StatusBadRequest, http.StatusNotFound}, - }, - { - Code: ErrorCodeNameUnknown, - Value: "NAME_UNKNOWN", - Message: "repository name not known to registry", - Description: `This is returned if the name used during an operation is - unknown to the registry.`, - HTTPStatusCodes: []int{http.StatusNotFound}, - }, - { - Code: ErrorCodeManifestUnknown, - Value: "MANIFEST_UNKNOWN", - Message: "manifest unknown", - Description: `This error is returned when the manifest, identified by - name and tag is unknown to the repository.`, - HTTPStatusCodes: []int{http.StatusNotFound}, - }, - { - Code: ErrorCodeManifestInvalid, - Value: "MANIFEST_INVALID", - Message: "manifest invalid", - Description: `During upload, manifests undergo several checks ensuring - validity. If those checks fail, this error may be returned, unless a - more specific error is included. The detail will contain information - the failed validation.`, - HTTPStatusCodes: []int{http.StatusBadRequest}, - }, - { - Code: ErrorCodeManifestUnverified, - Value: "MANIFEST_UNVERIFIED", - Message: "manifest failed signature verification", - Description: `During manifest upload, if the manifest fails signature - verification, this error will be returned.`, - HTTPStatusCodes: []int{http.StatusBadRequest}, - }, - { - Code: ErrorCodeBlobUnknown, - Value: "BLOB_UNKNOWN", - Message: "blob unknown to registry", - Description: `This error may be returned when a blob is unknown to the - registry in a specified repository. This can be returned with a - standard get or if a manifest references an unknown layer during - upload.`, - HTTPStatusCodes: []int{http.StatusBadRequest, http.StatusNotFound}, - }, - - { - Code: ErrorCodeBlobUploadUnknown, - Value: "BLOB_UPLOAD_UNKNOWN", - Message: "blob upload unknown to registry", - Description: `If a blob upload has been cancelled or was never - started, this error code may be returned.`, - HTTPStatusCodes: []int{http.StatusNotFound}, - }, - { - Code: ErrorCodeBlobUploadInvalid, - Value: "BLOB_UPLOAD_INVALID", - Message: "blob upload invalid", - Description: `The blob upload encountered an error and can no - longer proceed.`, - HTTPStatusCodes: []int{http.StatusNotFound}, - }, -} - -var errorCodeToDescriptors map[ErrorCode]ErrorDescriptor -var idToDescriptors map[string]ErrorDescriptor var routeDescriptorsMap map[string]RouteDescriptor func init() { - errorCodeToDescriptors = make(map[ErrorCode]ErrorDescriptor, len(errorDescriptors)) - idToDescriptors = make(map[string]ErrorDescriptor, len(errorDescriptors)) routeDescriptorsMap = make(map[string]RouteDescriptor, len(routeDescriptors)) - for _, descriptor := range errorDescriptors { - errorCodeToDescriptors[descriptor.Code] = descriptor - idToDescriptors[descriptor.Value] = descriptor - } for _, descriptor := range routeDescriptors { routeDescriptorsMap[descriptor.Name] = descriptor } diff --git a/registry/api/v2/errors.go b/registry/api/v2/errors.go index cbae020ef..fc61549ba 100644 --- a/registry/api/v2/errors.go +++ b/registry/api/v2/errors.go @@ -1,20 +1,14 @@ package v2 import ( - "fmt" - "strings" + "net/http" + + "github.com/docker/distribution/registry/api/errcode" ) -// ErrorCode represents the error type. The errors are serialized via strings -// and the integer format may change and should *never* be exported. -type ErrorCode int - const ( - // ErrorCodeUnknown is a catch-all for errors not defined below. - ErrorCodeUnknown ErrorCode = iota - // ErrorCodeUnsupported is returned when an operation is not supported. - ErrorCodeUnsupported + ErrorCodeUnsupported = iota // ErrorCodeUnauthorized is returned if a request is not authorized. ErrorCodeUnauthorized @@ -50,6 +44,10 @@ const ( // signature verfication. ErrorCodeManifestUnverified + // ErrorCodeManifestBlobUnknown is returned when a manifest blob is + // unknown to the registry. + ErrorCodeManifestBlobUnknown + // ErrorCodeBlobUnknown is returned when a blob is unknown to the // registry. This can happen when the manifest references a nonexistent // layer or the result is not found by a blob fetch. @@ -62,133 +60,133 @@ const ( ErrorCodeBlobUploadInvalid ) -// ParseErrorCode attempts to parse the error code string, returning -// ErrorCodeUnknown if the error is not known. -func ParseErrorCode(s string) ErrorCode { - desc, ok := idToDescriptors[s] +// ErrorDescriptors provides a list of HTTP API Error codes that may be +// encountered when interacting with the registry API. +var errorDescriptors = []errcode.ErrorDescriptor{ + { + Code: ErrorCodeUnsupported, + Value: "UNSUPPORTED", + Message: "The operation is unsupported.", + Description: `The operation was unsupported due to a missing + implementation or invalid set of parameters.`, + }, + { + Code: ErrorCodeUnauthorized, + Value: "UNAUTHORIZED", + Message: "access to the requested resource is not authorized", + Description: `The access controller denied access for the operation on + a resource. Often this will be accompanied by a 401 Unauthorized + response status.`, + HTTPStatusCode: http.StatusForbidden, + }, + { + Code: ErrorCodeDigestInvalid, + Value: "DIGEST_INVALID", + Message: "provided digest did not match uploaded content", + Description: `When a blob is uploaded, the registry will check that + the content matches the digest provided by the client. The error may + include a detail structure with the key "digest", including the + invalid digest string. This error may also be returned when a manifest + includes an invalid layer digest.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeSizeInvalid, + Value: "SIZE_INVALID", + Message: "provided length did not match content length", + Description: `When a layer is uploaded, the provided size will be + checked against the uploaded content. If they do not match, this error + will be returned.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeNameInvalid, + Value: "NAME_INVALID", + Message: "invalid repository name", + Description: `Invalid repository name encountered either during + manifest validation or any API operation.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeTagInvalid, + Value: "TAG_INVALID", + Message: "manifest tag did not match URI", + Description: `During a manifest upload, if the tag in the manifest + does not match the uri tag, this error will be returned.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeNameUnknown, + Value: "NAME_UNKNOWN", + Message: "repository name not known to registry", + Description: `This is returned if the name used during an operation is + unknown to the registry.`, + HTTPStatusCode: http.StatusNotFound, + }, + { + Code: ErrorCodeManifestUnknown, + Value: "MANIFEST_UNKNOWN", + Message: "manifest unknown", + Description: `This error is returned when the manifest, identified by + name and tag is unknown to the repository.`, + HTTPStatusCode: http.StatusNotFound, + }, + { + Code: ErrorCodeManifestInvalid, + Value: "MANIFEST_INVALID", + Message: "manifest invalid", + Description: `During upload, manifests undergo several checks ensuring + validity. If those checks fail, this error may be returned, unless a + more specific error is included. The detail will contain information + the failed validation.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeManifestUnverified, + Value: "MANIFEST_UNVERIFIED", + Message: "manifest failed signature verification", + Description: `During manifest upload, if the manifest fails signature + verification, this error will be returned.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeManifestBlobUnknown, + Value: "MANIFEST_BLOB_UNKNOWN", + Message: "blob unknown to registry", + Description: `This error may be returned when a manifest blob is + unknown to the registry.`, + HTTPStatusCode: http.StatusBadRequest, + }, + { + Code: ErrorCodeBlobUnknown, + Value: "BLOB_UNKNOWN", + Message: "blob unknown to registry", + Description: `This error may be returned when a blob is unknown to the + registry in a specified repository. This can be returned with a + standard get or if a manifest references an unknown layer during + upload.`, + HTTPStatusCode: http.StatusNotFound, + }, - if !ok { - return ErrorCodeUnknown - } - - return desc.Code + { + Code: ErrorCodeBlobUploadUnknown, + Value: "BLOB_UPLOAD_UNKNOWN", + Message: "blob upload unknown to registry", + Description: `If a blob upload has been cancelled or was never + started, this error code may be returned.`, + HTTPStatusCode: http.StatusNotFound, + }, + { + Code: ErrorCodeBlobUploadInvalid, + Value: "BLOB_UPLOAD_INVALID", + Message: "blob upload invalid", + Description: `The blob upload encountered an error and can no + longer proceed.`, + HTTPStatusCode: http.StatusNotFound, + }, } -// Descriptor returns the descriptor for the error code. -func (ec ErrorCode) Descriptor() ErrorDescriptor { - d, ok := errorCodeToDescriptors[ec] - - if !ok { - return ErrorCodeUnknown.Descriptor() - } - - return d -} - -// String returns the canonical identifier for this error code. -func (ec ErrorCode) String() string { - return ec.Descriptor().Value -} - -// Message returned the human-readable error message for this error code. -func (ec ErrorCode) Message() string { - return ec.Descriptor().Message -} - -// MarshalText encodes the receiver into UTF-8-encoded text and returns the -// result. -func (ec ErrorCode) MarshalText() (text []byte, err error) { - return []byte(ec.String()), nil -} - -// UnmarshalText decodes the form generated by MarshalText. -func (ec *ErrorCode) UnmarshalText(text []byte) error { - desc, ok := idToDescriptors[string(text)] - - if !ok { - desc = ErrorCodeUnknown.Descriptor() - } - - *ec = desc.Code - - return nil -} - -// Error provides a wrapper around ErrorCode with extra Details provided. -type Error struct { - Code ErrorCode `json:"code"` - Message string `json:"message,omitempty"` - Detail interface{} `json:"detail,omitempty"` -} - -// Error returns a human readable representation of the error. -func (e Error) Error() string { - return fmt.Sprintf("%s: %s", - strings.ToLower(strings.Replace(e.Code.String(), "_", " ", -1)), - e.Message) -} - -// Errors provides the envelope for multiple errors and a few sugar methods -// for use within the application. -type Errors struct { - Errors []Error `json:"errors,omitempty"` -} - -// Push pushes an error on to the error stack, with the optional detail -// argument. It is a programming error (ie panic) to push more than one -// detail at a time. -func (errs *Errors) Push(code ErrorCode, details ...interface{}) { - if len(details) > 1 { - panic("please specify zero or one detail items for this error") - } - - var detail interface{} - if len(details) > 0 { - detail = details[0] - } - - if err, ok := detail.(error); ok { - detail = err.Error() - } - - errs.PushErr(Error{ - Code: code, - Message: code.Message(), - Detail: detail, - }) -} - -// PushErr pushes an error interface onto the error stack. -func (errs *Errors) PushErr(err error) { - switch err.(type) { - case Error: - errs.Errors = append(errs.Errors, err.(Error)) - default: - errs.Errors = append(errs.Errors, Error{Message: err.Error()}) - } -} - -func (errs *Errors) Error() string { - switch errs.Len() { - case 0: - return "" - case 1: - return errs.Errors[0].Error() - default: - msg := "errors:\n" - for _, err := range errs.Errors { - msg += err.Error() + "\n" - } - return msg - } -} - -// Clear clears the errors. -func (errs *Errors) Clear() { - errs.Errors = errs.Errors[:0] -} - -// Len returns the current number of errors. -func (errs *Errors) Len() int { - return len(errs.Errors) +// init registers our errors with the errcode system +func init() { + errcode.LoadErrors(&errorDescriptors) } diff --git a/registry/client/blob_writer_test.go b/registry/client/blob_writer_test.go index 674d6e01b..3fdeb6ee3 100644 --- a/registry/client/blob_writer_test.go +++ b/registry/client/blob_writer_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/docker/distribution" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/testutil" ) @@ -161,14 +162,14 @@ func TestUploadReadFrom(t *testing.T) { if err == nil { t.Fatalf("Expected error when not found") } - if uploadErr, ok := err.(*v2.Errors); !ok { + if uploadErr, ok := err.(*errcode.Errors); !ok { t.Fatalf("Wrong error type %T: %s", err, err) } else if len(uploadErr.Errors) != 1 { t.Fatalf("Unexpected number of errors: %d, expected 1", len(uploadErr.Errors)) } else { v2Err := uploadErr.Errors[0] if v2Err.Code != v2.ErrorCodeBlobUploadInvalid { - t.Fatalf("Unexpected error code: %s, expected %s", v2Err.Code.String(), v2.ErrorCodeBlobUploadInvalid.String()) + t.Fatalf("Unexpected error code: %s, expected %d", v2Err.Code.String(), v2.ErrorCodeBlobUploadInvalid) } if expected := "invalid upload identifier"; v2Err.Message != expected { t.Fatalf("Unexpected error message: %s, expected %s", v2Err.Message, expected) diff --git a/registry/client/errors.go b/registry/client/errors.go index 2638055df..ef25dddf0 100644 --- a/registry/client/errors.go +++ b/registry/client/errors.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "net/http" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" ) @@ -32,7 +33,7 @@ func (e *UnexpectedHTTPResponseError) Error() string { } func parseHTTPErrorResponse(r io.Reader) error { - var errors v2.Errors + var errors errcode.Errors body, err := ioutil.ReadAll(r) if err != nil { return err @@ -51,7 +52,7 @@ func handleErrorResponse(resp *http.Response) error { if resp.StatusCode == 401 { err := parseHTTPErrorResponse(resp.Body) if uErr, ok := err.(*UnexpectedHTTPResponseError); ok { - return &v2.Error{ + return &errcode.Error{ Code: v2.ErrorCodeUnauthorized, Message: "401 Unauthorized", Detail: uErr.Response, diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index 6551c4920..24946ed5f 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -18,6 +18,7 @@ import ( "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/testutil" ) @@ -668,7 +669,7 @@ func TestManifestUnauthorized(t *testing.T) { if err == nil { t.Fatal("Expected error fetching manifest") } - v2Err, ok := err.(*v2.Error) + v2Err, ok := err.(*errcode.Error) if !ok { t.Fatalf("Unexpected error type: %#v", err) } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 5132f72e7..c5a994537 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -19,6 +19,7 @@ import ( "github.com/docker/distribution/configuration" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" _ "github.com/docker/distribution/registry/storage/driver/inmemory" "github.com/docker/distribution/testutil" @@ -373,7 +374,7 @@ func TestManifestAPI(t *testing.T) { _, p, counts := checkBodyHasErrorCodes(t, "getting unknown manifest tags", resp, v2.ErrorCodeManifestUnverified, v2.ErrorCodeBlobUnknown, v2.ErrorCodeDigestInvalid) - expectedCounts := map[v2.ErrorCode]int{ + expectedCounts := map[errcode.ErrorCode]int{ v2.ErrorCodeManifestUnverified: 1, v2.ErrorCodeBlobUnknown: 2, v2.ErrorCodeDigestInvalid: 2, @@ -748,13 +749,13 @@ func checkResponse(t *testing.T, msg string, resp *http.Response, expectedStatus // checkBodyHasErrorCodes ensures the body is an error body and has the // expected error codes, returning the error structure, the json slice and a // count of the errors by code. -func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, errorCodes ...v2.ErrorCode) (v2.Errors, []byte, map[v2.ErrorCode]int) { +func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, errorCodes ...errcode.ErrorCode) (errcode.Errors, []byte, map[errcode.ErrorCode]int) { p, err := ioutil.ReadAll(resp.Body) if err != nil { t.Fatalf("unexpected error reading body %s: %v", msg, err) } - var errs v2.Errors + var errs errcode.Errors if err := json.Unmarshal(p, &errs); err != nil { t.Fatalf("unexpected error decoding error response: %v", err) } @@ -770,8 +771,8 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error // resp.Header.Get("Content-Type")) // } - expected := map[v2.ErrorCode]struct{}{} - counts := map[v2.ErrorCode]int{} + expected := map[errcode.ErrorCode]struct{}{} + counts := map[errcode.ErrorCode]int{} // Initialize map with zeros for expected for _, code := range errorCodes { diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 1d58e9454..2747ac8b1 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -13,6 +13,7 @@ import ( "github.com/docker/distribution/configuration" ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/notifications" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/auth" registrymiddleware "github.com/docker/distribution/registry/middleware/registry" @@ -350,7 +351,6 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Errors.Push(v2.ErrorCodeNameInvalid, err) } - w.WriteHeader(http.StatusBadRequest) serveJSON(w, context.Errors) return } @@ -363,8 +363,8 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Repository, err = applyRepoMiddleware(context.Repository, app.Config.Middleware["repository"]) if err != nil { ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err) - context.Errors.Push(v2.ErrorCodeUnknown, err) - w.WriteHeader(http.StatusInternalServerError) + context.Errors.Push(errcode.ErrorCodeUnknown, err) + serveJSON(w, context.Errors) return } @@ -375,19 +375,14 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { // own errors if they need different behavior (such as range errors // for layer upload). if context.Errors.Len() > 0 { - if context.Value("http.response.status") == 0 { - // TODO(stevvooe): Getting this value from the context is a - // bit of a hack. We can further address with some of our - // future refactoring. - w.WriteHeader(http.StatusBadRequest) - } app.logError(context, context.Errors) + serveJSON(w, context.Errors) } }) } -func (app *App) logError(context context.Context, errors v2.Errors) { +func (app *App) logError(context context.Context, errors errcode.Errors) { for _, e := range errors.Errors { c := ctxu.WithValue(context, "err.code", e.Code) c = ctxu.WithValue(c, "err.message", e.Message) @@ -444,11 +439,10 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont // base route is accessed. This section prevents us from making // that mistake elsewhere in the code, allowing any operation to // proceed. - w.Header().Set("Content-Type", "application/json; charset=utf-8") - w.WriteHeader(http.StatusForbidden) - var errs v2.Errors + var errs errcode.Errors errs.Push(v2.ErrorCodeUnauthorized) + serveJSON(w, errs) return fmt.Errorf("forbidden: no repository name") } @@ -458,10 +452,18 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont if err != nil { switch err := err.(type) { case auth.Challenge: + // Since err.ServeHTTP will set the HTTP status code for us + // we need to set the content-type here. The serveJSON + // func will try to do it but it'll be too late at that point. + // I would have have preferred to just have the auth.Challenge + // ServerHTTP func just add the WWW-Authenticate header and let + // serveJSON set the HTTP status code and content-type but I wasn't + // sure if that's an ok design change. STEVVOOE ? w.Header().Set("Content-Type", "application/json; charset=utf-8") + err.ServeHTTP(w, r) - var errs v2.Errors + var errs errcode.Errors errs.Push(v2.ErrorCodeUnauthorized, accessRecords) serveJSON(w, errs) default: diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index fd1c486cb..da76dc0de 100644 --- a/registry/handlers/app_test.go +++ b/registry/handlers/app_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/docker/distribution/configuration" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/auth" _ "github.com/docker/distribution/registry/auth/silly" @@ -185,16 +186,18 @@ func TestNewApp(t *testing.T) { t.Fatalf("unexpected status code during request: %v", err) } - if req.Header.Get("Content-Type") != "application/json; charset=utf-8" { - t.Fatalf("unexpected content-type: %v != %v", req.Header.Get("Content-Type"), "application/json; charset=utf-8") - } + /* + if req.Header.Get("Content-Type") != "application/json; charset=utf-8" { + t.Fatalf("unexpected content-type: %v != %v", req.Header.Get("Content-Type"), "application/json; charset=utf-8") + } + */ expectedAuthHeader := "Bearer realm=\"realm-test\",service=\"service-test\"" if e, a := expectedAuthHeader, req.Header.Get("WWW-Authenticate"); e != a { t.Fatalf("unexpected WWW-Authenticate header: %q != %q", e, a) } - var errs v2.Errors + var errs errcode.Errors dec := json.NewDecoder(req.Body) if err := dec.Decode(&errs); err != nil { t.Fatalf("error decoding error response: %v", err) diff --git a/registry/handlers/blob.go b/registry/handlers/blob.go index 3237b1951..56699fe9a 100644 --- a/registry/handlers/blob.go +++ b/registry/handlers/blob.go @@ -6,6 +6,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/gorilla/handlers" ) @@ -17,7 +18,6 @@ func blobDispatcher(ctx *Context, r *http.Request) http.Handler { if err == errDigestNotAvailable { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNotFound) ctx.Errors.Push(v2.ErrorCodeDigestInvalid, err) }) } @@ -53,17 +53,16 @@ func (bh *blobHandler) GetBlob(w http.ResponseWriter, r *http.Request) { desc, err := blobs.Stat(bh, bh.Digest) if err != nil { if err == distribution.ErrBlobUnknown { - w.WriteHeader(http.StatusNotFound) bh.Errors.Push(v2.ErrorCodeBlobUnknown, bh.Digest) } else { - bh.Errors.Push(v2.ErrorCodeUnknown, err) + bh.Errors.Push(errcode.ErrorCodeUnknown, err) } return } if err := blobs.ServeBlob(bh, w, r, desc.Digest); err != nil { context.GetLogger(bh).Debugf("unexpected error getting blob HTTP handler: %v", err) - bh.Errors.Push(v2.ErrorCodeUnknown, err) + bh.Errors.Push(errcode.ErrorCodeUnknown, err) return } } diff --git a/registry/handlers/blobupload.go b/registry/handlers/blobupload.go index 99a75698d..7046edd35 100644 --- a/registry/handlers/blobupload.go +++ b/registry/handlers/blobupload.go @@ -10,6 +10,7 @@ import ( "github.com/docker/distribution" ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/gorilla/handlers" ) @@ -36,7 +37,6 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if err != nil { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("error resolving upload: %v", err) - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) }) } @@ -45,7 +45,6 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if state.Name != ctx.Repository.Name() { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Name()) - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) }) } @@ -53,7 +52,6 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if state.UUID != buh.UUID { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched uuid in upload state: %q != %q", state.UUID, buh.UUID) - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) }) } @@ -64,14 +62,12 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { ctxu.GetLogger(ctx).Errorf("error resolving upload: %v", err) if err == distribution.ErrBlobUploadUnknown { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNotFound) buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown, err) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusInternalServerError) - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) }) } buh.Upload = upload @@ -85,7 +81,6 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { defer upload.Close() ctxu.GetLogger(ctx).Infof("error seeking blob upload: %v", err) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) upload.Cancel(buh) }) @@ -93,7 +88,6 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { defer upload.Close() ctxu.GetLogger(ctx).Infof("seek to wrong offest: %d != %d", nn, buh.State.Offset) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) upload.Cancel(buh) }) @@ -125,8 +119,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req blobs := buh.Repository.Blobs(buh) upload, err := blobs.Create(buh) if err != nil { - w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } @@ -134,8 +127,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req defer buh.Upload.Close() if err := buh.blobUploadResponse(w, r, true); err != nil { - w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } @@ -146,7 +138,6 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req // GetUploadStatus returns the status of a given upload, identified by id. func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - w.WriteHeader(http.StatusNotFound) buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) return } @@ -155,8 +146,7 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // resumable upload is supported. This will enable returning a non-zero // range for clients to begin uploading at an offset. if err := buh.blobUploadResponse(w, r, true); err != nil { - w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } @@ -167,14 +157,13 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // PatchBlobData writes data to an upload. func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - w.WriteHeader(http.StatusNotFound) buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) return } ct := r.Header.Get("Content-Type") if ct != "" && ct != "application/octet-stream" { - w.WriteHeader(http.StatusBadRequest) + buh.Errors.Push(errcode.ErrorCodeUnknown, fmt.Errorf("Bad Content-Type")) // TODO(dmcgowan): encode error return } @@ -184,14 +173,12 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // Copy the data if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - w.WriteHeader(http.StatusInternalServerError) - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } if err := buh.blobUploadResponse(w, r, false); err != nil { - w.WriteHeader(http.StatusInternalServerError) // Error conditions here? - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } @@ -205,7 +192,6 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // url of the blob. func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - w.WriteHeader(http.StatusNotFound) buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) return } @@ -214,7 +200,6 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if dgstStr == "" { // no digest? return error, but allow retry. - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeDigestInvalid, "digest missing") return } @@ -222,7 +207,6 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht dgst, err := digest.ParseDigest(dgstStr) if err != nil { // no digest? return error, but allow retry. - w.WriteHeader(http.StatusNotFound) buh.Errors.Push(v2.ErrorCodeDigestInvalid, "digest parsing failed") return } @@ -230,8 +214,7 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // Read in the data, if any. if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - w.WriteHeader(http.StatusInternalServerError) - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } @@ -246,17 +229,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if err != nil { switch err := err.(type) { case distribution.ErrBlobInvalidDigest: - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeDigestInvalid, err) default: switch err { case distribution.ErrBlobInvalidLength, distribution.ErrBlobDigestUnsupported: - w.WriteHeader(http.StatusBadRequest) buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) default: ctxu.GetLogger(buh).Errorf("unknown error completing upload: %#v", err) - w.WriteHeader(http.StatusInternalServerError) - buh.Errors.Push(v2.ErrorCodeUnknown, err) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) } } @@ -273,8 +253,7 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // Build our canonical blob url blobURL, err := buh.urlBuilder.BuildBlobURL(buh.Repository.Name(), desc.Digest) if err != nil { - buh.Errors.Push(v2.ErrorCodeUnknown, err) - w.WriteHeader(http.StatusInternalServerError) + buh.Errors.Push(errcode.ErrorCodeUnknown, err) return } @@ -287,7 +266,6 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // CancelBlobUpload cancels an in-progress upload of a blob. func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - w.WriteHeader(http.StatusNotFound) buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) return } @@ -295,7 +273,6 @@ func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Re w.Header().Set("Docker-Upload-UUID", buh.UUID) if err := buh.Upload.Cancel(buh); err != nil { ctxu.GetLogger(buh).Errorf("error encountered canceling upload: %v", err) - w.WriteHeader(http.StatusInternalServerError) buh.Errors.PushErr(err) } diff --git a/registry/handlers/context.go b/registry/handlers/context.go index 0df553468..85a171237 100644 --- a/registry/handlers/context.go +++ b/registry/handlers/context.go @@ -8,6 +8,7 @@ import ( "github.com/docker/distribution" ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "golang.org/x/net/context" ) @@ -27,7 +28,7 @@ type Context struct { // Errors is a collection of errors encountered during the request to be // returned to the client API. If errors are added to the collection, the // handler *must not* start the response via http.ResponseWriter. - Errors v2.Errors + Errors errcode.Errors urlBuilder *v2.URLBuilder diff --git a/registry/handlers/helpers.go b/registry/handlers/helpers.go index f2879137b..3611a72d9 100644 --- a/registry/handlers/helpers.go +++ b/registry/handlers/helpers.go @@ -2,6 +2,7 @@ package handlers import ( "encoding/json" + "github.com/docker/distribution/registry/api/errcode" "io" "net/http" ) @@ -11,6 +12,17 @@ import ( // ResponseWriter.WriteHeader before this function. func serveJSON(w http.ResponseWriter, v interface{}) error { w.Header().Set("Content-Type", "application/json; charset=utf-8") + sc := http.StatusInternalServerError + + if errs, ok := v.(errcode.Errors); ok && errs.Len() > 0 { + sc = errs.Errors[0].Code.Descriptor().HTTPStatusCode + if sc == 0 { + sc = http.StatusInternalServerError + } + } + + w.WriteHeader(sc) + enc := json.NewEncoder(w) if err := enc.Encode(v); err != nil { diff --git a/registry/handlers/images.go b/registry/handlers/images.go index 45029da51..d717cf724 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -64,7 +64,6 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http if err != nil { imh.Errors.Push(v2.ErrorCodeManifestUnknown, err) - w.WriteHeader(http.StatusNotFound) return } @@ -73,7 +72,6 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http dgst, err := digestManifest(imh, sm) if err != nil { imh.Errors.Push(v2.ErrorCodeDigestInvalid, err) - w.WriteHeader(http.StatusBadRequest) return } @@ -95,14 +93,12 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http var manifest manifest.SignedManifest if err := dec.Decode(&manifest); err != nil { imh.Errors.Push(v2.ErrorCodeManifestInvalid, err) - w.WriteHeader(http.StatusBadRequest) return } dgst, err := digestManifest(imh, &manifest) if err != nil { imh.Errors.Push(v2.ErrorCodeDigestInvalid, err) - w.WriteHeader(http.StatusBadRequest) return } @@ -111,7 +107,6 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http if manifest.Tag != imh.Tag { ctxu.GetLogger(imh).Errorf("invalid tag on manifest payload: %q != %q", manifest.Tag, imh.Tag) imh.Errors.Push(v2.ErrorCodeTagInvalid) - w.WriteHeader(http.StatusBadRequest) return } @@ -120,12 +115,10 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http if dgst != imh.Digest { ctxu.GetLogger(imh).Errorf("payload digest does match: %q != %q", dgst, imh.Digest) imh.Errors.Push(v2.ErrorCodeDigestInvalid) - w.WriteHeader(http.StatusBadRequest) return } } else { imh.Errors.Push(v2.ErrorCodeTagInvalid, "no tag or digest specified") - w.WriteHeader(http.StatusBadRequest) return } @@ -152,7 +145,6 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http imh.Errors.PushErr(err) } - w.WriteHeader(http.StatusBadRequest) return } @@ -180,7 +172,6 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h // Once we work out schema version 2, the full deletion system will be // worked out and we can add support back. imh.Errors.Push(v2.ErrorCodeUnsupported) - w.WriteHeader(http.StatusBadRequest) } // digestManifest takes a digest of the given manifest. This belongs somewhere diff --git a/registry/handlers/tags.go b/registry/handlers/tags.go index be84fae58..44b12dfdb 100644 --- a/registry/handlers/tags.go +++ b/registry/handlers/tags.go @@ -39,7 +39,6 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { if err != nil { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - w.WriteHeader(404) th.Errors.Push(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Repository.Name()}) default: th.Errors.PushErr(err) From 8a0827f7999f07e54053fbbe9173159b56aa6e76 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Tue, 26 May 2015 17:18:32 -0700 Subject: [PATCH 2/4] Round 2 Make Errors a []Error Signed-off-by: Doug Davis --- registry/api/errcode/errors.go | 62 +++++++++++------------------ registry/api/v2/errors.go | 2 +- registry/client/blob_writer_test.go | 19 ++++----- registry/client/errors.go | 7 ++-- registry/client/repository_test.go | 4 +- registry/handlers/api_test.go | 4 +- registry/handlers/app.go | 14 +++---- registry/handlers/app_test.go | 4 +- registry/handlers/blob.go | 10 ++--- registry/handlers/blobupload.go | 50 +++++++++++------------ registry/handlers/helpers.go | 4 +- registry/handlers/images.go | 27 +++++++------ registry/handlers/tags.go | 7 ++-- 13 files changed, 98 insertions(+), 116 deletions(-) diff --git a/registry/api/errcode/errors.go b/registry/api/errcode/errors.go index ce3c06246..c46670a13 100644 --- a/registry/api/errcode/errors.go +++ b/registry/api/errcode/errors.go @@ -55,8 +55,8 @@ var errorDescriptors = []ErrorDescriptor{ } // LoadErrors will register a new set of Errors into the system -func LoadErrors(errs *[]ErrorDescriptor) { - for _, descriptor := range *errs { +func LoadErrors(errs []ErrorDescriptor) { + for _, descriptor := range errs { if _, ok := idToDescriptors[descriptor.Value]; ok { panic(fmt.Sprintf("ErrorValue %s is already registered", descriptor.Value)) } @@ -123,28 +123,28 @@ func (ec *ErrorCode) UnmarshalText(text []byte) error { // Error provides a wrapper around ErrorCode with extra Details provided. type Error struct { - Code ErrorCode `json:"code"` - Message string `json:"message,omitempty"` - Detail interface{} `json:"detail,omitempty"` + Code ErrorCode `json:"code"` + Detail interface{} `json:"detail,omitempty"` } // Error returns a human readable representation of the error. func (e Error) Error() string { return fmt.Sprintf("%s: %s", strings.ToLower(strings.Replace(e.Code.String(), "_", " ", -1)), - e.Message) + e.Code.Message()) +} + +// Message returned the human-readable error message for this Error +func (e Error) Message() string { + return e.Code.Message() } // Errors provides the envelope for multiple errors and a few sugar methods // for use within the application. -type Errors struct { - Errors []Error `json:"errors,omitempty"` -} +type Errors []Error -// Push pushes an error on to the error stack, with the optional detail -// argument. It is a programming error (ie panic) to push more than one -// detail at a time. -func (errs *Errors) Push(code ErrorCode, details ...interface{}) { +// NewError creates a new Error struct based on the passed-in info +func NewError(code ErrorCode, details ...interface{}) Error { if len(details) > 1 { panic("please specify zero or one detail items for this error") } @@ -158,49 +158,33 @@ func (errs *Errors) Push(code ErrorCode, details ...interface{}) { detail = err.Error() } - errs.PushErr(Error{ - Code: code, - Message: code.Message(), - Detail: detail, - }) -} - -// PushErr pushes an error interface onto the error stack. -func (errs *Errors) PushErr(err error) { - switch err.(type) { - case Error: - errs.Errors = append(errs.Errors, err.(Error)) - default: - errs.Errors = append(errs.Errors, Error{Message: err.Error()}) + return Error{ + Code: code, + Detail: detail, } } -func (errs *Errors) Error() string { - switch errs.Len() { +func (errs Errors) Error() string { + switch len(errs) { case 0: return "" case 1: - return errs.Errors[0].Error() + return errs[0].Error() default: msg := "errors:\n" - for _, err := range errs.Errors { + for _, err := range errs { msg += err.Error() + "\n" } return msg } } -// Clear clears the errors. -func (errs *Errors) Clear() { - errs.Errors = nil -} - // Len returns the current number of errors. -func (errs *Errors) Len() int { - return len(errs.Errors) +func (errs Errors) Len() int { + return len(errs) } // init loads the default errors that are part of the errcode package func init() { - LoadErrors(&errorDescriptors) + LoadErrors(errorDescriptors) } diff --git a/registry/api/v2/errors.go b/registry/api/v2/errors.go index fc61549ba..9655dba86 100644 --- a/registry/api/v2/errors.go +++ b/registry/api/v2/errors.go @@ -188,5 +188,5 @@ var errorDescriptors = []errcode.ErrorDescriptor{ // init registers our errors with the errcode system func init() { - errcode.LoadErrors(&errorDescriptors) + errcode.LoadErrors(errorDescriptors) } diff --git a/registry/client/blob_writer_test.go b/registry/client/blob_writer_test.go index 3fdeb6ee3..74545b065 100644 --- a/registry/client/blob_writer_test.go +++ b/registry/client/blob_writer_test.go @@ -86,15 +86,12 @@ func TestUploadReadFrom(t *testing.T) { Response: testutil.Response{ StatusCode: http.StatusBadRequest, Body: []byte(` - { - "errors": [ + [ { "code": "BLOB_UPLOAD_INVALID", - "message": "invalid upload identifier", "detail": "more detail" } - ] - }`), + ] `), }, }, // Test 400 invalid json @@ -162,17 +159,17 @@ func TestUploadReadFrom(t *testing.T) { if err == nil { t.Fatalf("Expected error when not found") } - if uploadErr, ok := err.(*errcode.Errors); !ok { + if uploadErr, ok := err.(errcode.Errors); !ok { t.Fatalf("Wrong error type %T: %s", err, err) - } else if len(uploadErr.Errors) != 1 { - t.Fatalf("Unexpected number of errors: %d, expected 1", len(uploadErr.Errors)) + } else if len(uploadErr) != 1 { + t.Fatalf("Unexpected number of errors: %d, expected 1", len(uploadErr)) } else { - v2Err := uploadErr.Errors[0] + v2Err := uploadErr[0] if v2Err.Code != v2.ErrorCodeBlobUploadInvalid { t.Fatalf("Unexpected error code: %s, expected %d", v2Err.Code.String(), v2.ErrorCodeBlobUploadInvalid) } - if expected := "invalid upload identifier"; v2Err.Message != expected { - t.Fatalf("Unexpected error message: %s, expected %s", v2Err.Message, expected) + if expected := "blob upload invalid"; v2Err.Message() != expected { + t.Fatalf("Unexpected error message: %s, expected %s", v2Err.Message(), expected) } if expected := "more detail"; v2Err.Detail.(string) != expected { t.Fatalf("Unexpected error message: %s, expected %s", v2Err.Detail.(string), expected) diff --git a/registry/client/errors.go b/registry/client/errors.go index ef25dddf0..e743533b9 100644 --- a/registry/client/errors.go +++ b/registry/client/errors.go @@ -45,7 +45,7 @@ func parseHTTPErrorResponse(r io.Reader) error { Response: body, } } - return &errors + return errors } func handleErrorResponse(resp *http.Response) error { @@ -53,9 +53,8 @@ func handleErrorResponse(resp *http.Response) error { err := parseHTTPErrorResponse(resp.Body) if uErr, ok := err.(*UnexpectedHTTPResponseError); ok { return &errcode.Error{ - Code: v2.ErrorCodeUnauthorized, - Message: "401 Unauthorized", - Detail: uErr.Response, + Code: v2.ErrorCodeUnauthorized, + Detail: uErr.Response, } } return err diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index 24946ed5f..7dbe97cf7 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -676,7 +676,7 @@ func TestManifestUnauthorized(t *testing.T) { if v2Err.Code != v2.ErrorCodeUnauthorized { t.Fatalf("Unexpected error code: %s", v2Err.Code.String()) } - if expected := "401 Unauthorized"; v2Err.Message != expected { - t.Fatalf("Unexpected message value: %s, expected %s", v2Err.Message, expected) + if expected := errcode.ErrorCode(v2.ErrorCodeUnauthorized).Message(); v2Err.Message() != expected { + t.Fatalf("Unexpected message value: %s, expected %s", v2Err.Message(), expected) } } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index c5a994537..146fcf4c9 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -760,7 +760,7 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error t.Fatalf("unexpected error decoding error response: %v", err) } - if len(errs.Errors) == 0 { + if len(errs) == 0 { t.Fatalf("expected errors in response") } @@ -780,7 +780,7 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error counts[code] = 0 } - for _, err := range errs.Errors { + for _, err := range errs { if _, ok := expected[err.Code]; !ok { t.Fatalf("unexpected error code %v encountered during %s: %s ", err.Code, msg, string(p)) } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 2747ac8b1..12c6e2274 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -346,9 +346,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - context.Errors.Push(v2.ErrorCodeNameUnknown, err) + context.Errors = append(context.Errors, errcode.NewError(v2.ErrorCodeNameUnknown, err)) case distribution.ErrRepositoryNameInvalid: - context.Errors.Push(v2.ErrorCodeNameInvalid, err) + context.Errors = append(context.Errors, errcode.NewError(v2.ErrorCodeNameInvalid, err)) } serveJSON(w, context.Errors) @@ -363,7 +363,7 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Repository, err = applyRepoMiddleware(context.Repository, app.Config.Middleware["repository"]) if err != nil { ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err) - context.Errors.Push(errcode.ErrorCodeUnknown, err) + context.Errors = append(context.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) serveJSON(w, context.Errors) return @@ -383,9 +383,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { } func (app *App) logError(context context.Context, errors errcode.Errors) { - for _, e := range errors.Errors { + for _, e := range errors { c := ctxu.WithValue(context, "err.code", e.Code) - c = ctxu.WithValue(c, "err.message", e.Message) + c = ctxu.WithValue(c, "err.message", e.Code.Message()) c = ctxu.WithValue(c, "err.detail", e.Detail) c = ctxu.WithLogger(c, ctxu.GetLogger(c, "err.code", @@ -441,7 +441,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont // proceed. var errs errcode.Errors - errs.Push(v2.ErrorCodeUnauthorized) + errs = append(errs, errcode.NewError(v2.ErrorCodeUnauthorized)) serveJSON(w, errs) return fmt.Errorf("forbidden: no repository name") @@ -464,7 +464,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont err.ServeHTTP(w, r) var errs errcode.Errors - errs.Push(v2.ErrorCodeUnauthorized, accessRecords) + errs = append(errs, errcode.NewError(v2.ErrorCodeUnauthorized, accessRecords)) serveJSON(w, errs) default: // This condition is a potential security problem either in diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index da76dc0de..0520cb403 100644 --- a/registry/handlers/app_test.go +++ b/registry/handlers/app_test.go @@ -203,8 +203,8 @@ func TestNewApp(t *testing.T) { t.Fatalf("error decoding error response: %v", err) } - if errs.Errors[0].Code != v2.ErrorCodeUnauthorized { - t.Fatalf("unexpected error code: %v != %v", errs.Errors[0].Code, v2.ErrorCodeUnauthorized) + if errs[0].Code != v2.ErrorCodeUnauthorized { + t.Fatalf("unexpected error code: %v != %v", errs[0].Code, v2.ErrorCodeUnauthorized) } } diff --git a/registry/handlers/blob.go b/registry/handlers/blob.go index 56699fe9a..fa9f576aa 100644 --- a/registry/handlers/blob.go +++ b/registry/handlers/blob.go @@ -18,12 +18,12 @@ func blobDispatcher(ctx *Context, r *http.Request) http.Handler { if err == errDigestNotAvailable { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors.Push(v2.ErrorCodeDigestInvalid, err) + ctx.Errors = append(ctx.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors.Push(v2.ErrorCodeDigestInvalid, err) + ctx.Errors = append(ctx.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) }) } @@ -53,16 +53,16 @@ func (bh *blobHandler) GetBlob(w http.ResponseWriter, r *http.Request) { desc, err := blobs.Stat(bh, bh.Digest) if err != nil { if err == distribution.ErrBlobUnknown { - bh.Errors.Push(v2.ErrorCodeBlobUnknown, bh.Digest) + bh.Errors = append(bh.Errors, errcode.NewError(v2.ErrorCodeBlobUnknown, bh.Digest)) } else { - bh.Errors.Push(errcode.ErrorCodeUnknown, err) + bh.Errors = append(bh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) } return } if err := blobs.ServeBlob(bh, w, r, desc.Digest); err != nil { context.GetLogger(bh).Debugf("unexpected error getting blob HTTP handler: %v", err) - bh.Errors.Push(errcode.ErrorCodeUnknown, err) + bh.Errors = append(bh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } } diff --git a/registry/handlers/blobupload.go b/registry/handlers/blobupload.go index 7046edd35..7e8c39622 100644 --- a/registry/handlers/blobupload.go +++ b/registry/handlers/blobupload.go @@ -37,7 +37,7 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if err != nil { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("error resolving upload: %v", err) - buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) }) } buh.State = state @@ -45,14 +45,14 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if state.Name != ctx.Repository.Name() { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Name()) - buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) }) } if state.UUID != buh.UUID { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched uuid in upload state: %q != %q", state.UUID, buh.UUID) - buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) }) } @@ -62,12 +62,12 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { ctxu.GetLogger(ctx).Errorf("error resolving upload: %v", err) if err == distribution.ErrBlobUploadUnknown { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown, err)) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) }) } buh.Upload = upload @@ -81,14 +81,14 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { defer upload.Close() ctxu.GetLogger(ctx).Infof("error seeking blob upload: %v", err) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) upload.Cancel(buh) }) } else if nn != buh.State.Offset { defer upload.Close() ctxu.GetLogger(ctx).Infof("seek to wrong offest: %d != %d", nn, buh.State.Offset) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) upload.Cancel(buh) }) } @@ -119,7 +119,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req blobs := buh.Repository.Blobs(buh) upload, err := blobs.Create(buh) if err != nil { - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } @@ -127,7 +127,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req defer buh.Upload.Close() if err := buh.blobUploadResponse(w, r, true); err != nil { - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } @@ -138,7 +138,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req // GetUploadStatus returns the status of a given upload, identified by id. func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) return } @@ -146,7 +146,7 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // resumable upload is supported. This will enable returning a non-zero // range for clients to begin uploading at an offset. if err := buh.blobUploadResponse(w, r, true); err != nil { - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } @@ -157,13 +157,13 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // PatchBlobData writes data to an upload. func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) return } ct := r.Header.Get("Content-Type") if ct != "" && ct != "application/octet-stream" { - buh.Errors.Push(errcode.ErrorCodeUnknown, fmt.Errorf("Bad Content-Type")) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, fmt.Errorf("Bad Content-Type"))) // TODO(dmcgowan): encode error return } @@ -173,12 +173,12 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // Copy the data if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } if err := buh.blobUploadResponse(w, r, false); err != nil { - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } @@ -192,7 +192,7 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // url of the blob. func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) return } @@ -200,21 +200,21 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if dgstStr == "" { // no digest? return error, but allow retry. - buh.Errors.Push(v2.ErrorCodeDigestInvalid, "digest missing") + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, "digest missing")) return } dgst, err := digest.ParseDigest(dgstStr) if err != nil { // no digest? return error, but allow retry. - buh.Errors.Push(v2.ErrorCodeDigestInvalid, "digest parsing failed") + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, "digest parsing failed")) return } // Read in the data, if any. if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } @@ -229,14 +229,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if err != nil { switch err := err.(type) { case distribution.ErrBlobInvalidDigest: - buh.Errors.Push(v2.ErrorCodeDigestInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) default: switch err { case distribution.ErrBlobInvalidLength, distribution.ErrBlobDigestUnsupported: - buh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) default: ctxu.GetLogger(buh).Errorf("unknown error completing upload: %#v", err) - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) } } @@ -253,7 +253,7 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // Build our canonical blob url blobURL, err := buh.urlBuilder.BuildBlobURL(buh.Repository.Name(), desc.Digest) if err != nil { - buh.Errors.Push(errcode.ErrorCodeUnknown, err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } @@ -266,14 +266,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // CancelBlobUpload cancels an in-progress upload of a blob. func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors.Push(v2.ErrorCodeBlobUploadUnknown) + buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) return } w.Header().Set("Docker-Upload-UUID", buh.UUID) if err := buh.Upload.Cancel(buh); err != nil { ctxu.GetLogger(buh).Errorf("error encountered canceling upload: %v", err) - buh.Errors.PushErr(err) + buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) } w.WriteHeader(http.StatusNoContent) diff --git a/registry/handlers/helpers.go b/registry/handlers/helpers.go index 3611a72d9..f4f241751 100644 --- a/registry/handlers/helpers.go +++ b/registry/handlers/helpers.go @@ -14,8 +14,8 @@ func serveJSON(w http.ResponseWriter, v interface{}) error { w.Header().Set("Content-Type", "application/json; charset=utf-8") sc := http.StatusInternalServerError - if errs, ok := v.(errcode.Errors); ok && errs.Len() > 0 { - sc = errs.Errors[0].Code.Descriptor().HTTPStatusCode + if errs, ok := v.(errcode.Errors); ok && len(errs) > 0 { + sc = errs[0].Code.Descriptor().HTTPStatusCode if sc == 0 { sc = http.StatusInternalServerError } diff --git a/registry/handlers/images.go b/registry/handlers/images.go index d717cf724..9d025c787 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -10,6 +10,7 @@ import ( ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/gorilla/handlers" "golang.org/x/net/context" @@ -63,7 +64,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http } if err != nil { - imh.Errors.Push(v2.ErrorCodeManifestUnknown, err) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestUnknown, err)) return } @@ -71,7 +72,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http if imh.Digest == "" { dgst, err := digestManifest(imh, sm) if err != nil { - imh.Errors.Push(v2.ErrorCodeDigestInvalid, err) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) return } @@ -92,13 +93,13 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http var manifest manifest.SignedManifest if err := dec.Decode(&manifest); err != nil { - imh.Errors.Push(v2.ErrorCodeManifestInvalid, err) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestInvalid, err)) return } dgst, err := digestManifest(imh, &manifest) if err != nil { - imh.Errors.Push(v2.ErrorCodeDigestInvalid, err) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) return } @@ -106,7 +107,7 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http if imh.Tag != "" { if manifest.Tag != imh.Tag { ctxu.GetLogger(imh).Errorf("invalid tag on manifest payload: %q != %q", manifest.Tag, imh.Tag) - imh.Errors.Push(v2.ErrorCodeTagInvalid) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeTagInvalid)) return } @@ -114,11 +115,11 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http } else if imh.Digest != "" { if dgst != imh.Digest { ctxu.GetLogger(imh).Errorf("payload digest does match: %q != %q", dgst, imh.Digest) - imh.Errors.Push(v2.ErrorCodeDigestInvalid) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid)) return } } else { - imh.Errors.Push(v2.ErrorCodeTagInvalid, "no tag or digest specified") + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeTagInvalid, "no tag or digest specified")) return } @@ -130,19 +131,19 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http for _, verificationError := range err { switch verificationError := verificationError.(type) { case distribution.ErrManifestBlobUnknown: - imh.Errors.Push(v2.ErrorCodeBlobUnknown, verificationError.Digest) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeBlobUnknown, verificationError.Digest)) case distribution.ErrManifestUnverified: - imh.Errors.Push(v2.ErrorCodeManifestUnverified) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestUnverified)) default: if verificationError == digest.ErrDigestInvalidFormat { - imh.Errors.Push(v2.ErrorCodeDigestInvalid) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid)) } else { - imh.Errors.PushErr(verificationError) + imh.Errors = append(imh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, verificationError)) } } } default: - imh.Errors.PushErr(err) + imh.Errors = append(imh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) } return @@ -171,7 +172,7 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h // tag index entries a serious problem in eventually consistent storage. // Once we work out schema version 2, the full deletion system will be // worked out and we can add support back. - imh.Errors.Push(v2.ErrorCodeUnsupported) + imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeUnsupported)) } // digestManifest takes a digest of the given manifest. This belongs somewhere diff --git a/registry/handlers/tags.go b/registry/handlers/tags.go index 44b12dfdb..e1846cf96 100644 --- a/registry/handlers/tags.go +++ b/registry/handlers/tags.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/docker/distribution" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" "github.com/gorilla/handlers" ) @@ -39,9 +40,9 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { if err != nil { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - th.Errors.Push(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Repository.Name()}) + th.Errors = append(th.Errors, errcode.NewError(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Repository.Name()})) default: - th.Errors.PushErr(err) + th.Errors = append(th.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) } return } @@ -53,7 +54,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { Name: th.Repository.Name(), Tags: tags, }); err != nil { - th.Errors.PushErr(err) + th.Errors = append(th.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) return } } From 38393b63b770f34bd0b65e92649a44513100771b Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Tue, 26 May 2015 18:16:45 -0700 Subject: [PATCH 3/4] Round 3 - Add Register function Signed-off-by: Doug Davis --- registry/api/errcode/errors.go | 97 +++++++++------- registry/api/errcode/errors_test.go | 156 ++++++++++---------------- registry/api/v2/descriptors.go | 5 - registry/api/v2/errors.go | 168 +++++++++++----------------- registry/handlers/app.go | 1 + registry/handlers/app_test.go | 8 +- registry/handlers/helpers.go | 3 +- 7 files changed, 187 insertions(+), 251 deletions(-) diff --git a/registry/api/errcode/errors.go b/registry/api/errcode/errors.go index c46670a13..4285dedc7 100644 --- a/registry/api/errcode/errors.go +++ b/registry/api/errcode/errors.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "strings" + "sync" ) // ErrorCode represents the error type. The errors are serialized via strings @@ -36,49 +37,70 @@ type ErrorDescriptor struct { var ( errorCodeToDescriptors = map[ErrorCode]ErrorDescriptor{} idToDescriptors = map[string]ErrorDescriptor{} + groupToDescriptors = map[string][]ErrorDescriptor{} ) -const ( - // ErrorCodeUnknown is a catch-all for errors not defined below. - ErrorCodeUnknown ErrorCode = 10000 + iota -) - -var errorDescriptors = []ErrorDescriptor{ - { - Code: ErrorCodeUnknown, - Value: "UNKNOWN", - Message: "unknown error", - Description: `Generic error returned when the error does not have an +// ErrorCodeUnknown is a generic error that can be used as a last +// resort if there is no situation-specific error message that can be used +var ErrorCodeUnknown = Register("registry.api.errcode", ErrorDescriptor{ + Value: "UNKNOWN", + Message: "unknown error", + Description: `Generic error returned when the error does not have an API classification.`, - HTTPStatusCode: http.StatusInternalServerError, - }, -} + HTTPStatusCode: http.StatusInternalServerError, +}) -// LoadErrors will register a new set of Errors into the system -func LoadErrors(errs []ErrorDescriptor) { - for _, descriptor := range errs { - if _, ok := idToDescriptors[descriptor.Value]; ok { - panic(fmt.Sprintf("ErrorValue %s is already registered", descriptor.Value)) - } - if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { - panic(fmt.Sprintf("ErrorCode %d is already registered", descriptor.Code)) - } +var nextCode = 1000 +var registerLock sync.Mutex - errorCodeToDescriptors[descriptor.Code] = descriptor - idToDescriptors[descriptor.Value] = descriptor +// Register will make the passed-in error known to the environment and +// return a new ErrorCode +func Register(group string, descriptor ErrorDescriptor) ErrorCode { + registerLock.Lock() + defer registerLock.Unlock() + code := ErrorCode(nextCode) + + descriptor.Code = code + + if _, ok := idToDescriptors[descriptor.Value]; ok { + panic(fmt.Sprintf("ErrorValue %s is already registered", descriptor.Value)) } -} - -// ParseErrorCode attempts to parse the error code string, returning -// ErrorCodeUnknown if the error is not known. -func ParseErrorCode(s string) ErrorCode { - desc, ok := idToDescriptors[s] - - if !ok { - return ErrorCodeUnknown + if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { + panic(fmt.Sprintf("ErrorCode %d is already registered", descriptor.Code)) } - return desc.Code + groupToDescriptors[group] = append(groupToDescriptors[group], descriptor) + errorCodeToDescriptors[code] = descriptor + idToDescriptors[descriptor.Value] = descriptor + + nextCode++ + return code +} + +// ParseErrorCode returns the value by the string error code. +// `ErrorCodeUnknown` will be returned if the error is not known. +func ParseErrorCode(value string) ErrorCode { + ed, ok := idToDescriptors[value] + if ok { + return ed.Code + } + + return ErrorCodeUnknown +} + +// GetGroupNames returns the list of Error group names that are registered +func GetGroupNames() []string { + keys := []string{} + + for k := range groupToDescriptors { + keys = append(keys, k) + } + return keys +} + +// GetErrorCodeGroup returns the named group of error descriptors +func GetErrorCodeGroup(name string) []ErrorDescriptor { + return groupToDescriptors[name] } // Descriptor returns the descriptor for the error code. @@ -183,8 +205,3 @@ func (errs Errors) Error() string { func (errs Errors) Len() int { return len(errs) } - -// init loads the default errors that are part of the errcode package -func init() { - LoadErrors(errorDescriptors) -} diff --git a/registry/api/errcode/errors_test.go b/registry/api/errcode/errors_test.go index eedb22ed4..aaf0d73b7 100644 --- a/registry/api/errcode/errors_test.go +++ b/registry/api/errcode/errors_test.go @@ -2,67 +2,86 @@ package errcode import ( "encoding/json" - // "reflect" + "net/http" + "reflect" "testing" - - // "github.com/docker/distribution/digest" ) // TestErrorCodes ensures that error code format, mappings and // marshaling/unmarshaling. round trips are stable. func TestErrorCodes(t *testing.T) { - for _, desc := range errorDescriptors { - if desc.Code.String() != desc.Value { - t.Fatalf("error code string incorrect: %q != %q", desc.Code.String(), desc.Value) + if len(errorCodeToDescriptors) == 0 { + t.Fatal("errors aren't loaded!") + } + + for ec, desc := range errorCodeToDescriptors { + if ec != desc.Code { + t.Fatalf("error code in descriptor isn't correct, %q != %q", ec, desc.Code) } - if desc.Code.Message() != desc.Message { - t.Fatalf("incorrect message for error code %v: %q != %q", desc.Code, desc.Code.Message(), desc.Message) + if idToDescriptors[desc.Value].Code != ec { + t.Fatalf("error code in idToDesc isn't correct, %q != %q", idToDescriptors[desc.Value].Code, ec) } - // Serialize the error code using the json library to ensure that we - // get a string and it works round trip. - p, err := json.Marshal(desc.Code) + if ec.Message() != desc.Message { + t.Fatalf("ec.Message doesn't mtach desc.Message: %q != %q", ec.Message(), desc.Message) + } + // Test (de)serializing the ErrorCode + p, err := json.Marshal(ec) if err != nil { - t.Fatalf("error marshaling error code %v: %v", desc.Code, err) + t.Fatalf("couldn't marshal ec %v: %v", ec, err) } if len(p) <= 0 { - t.Fatalf("expected content in marshaled before for error code %v", desc.Code) + t.Fatalf("expected content in marshaled before for error code %v", ec) } // First, unmarshal to interface and ensure we have a string. var ecUnspecified interface{} if err := json.Unmarshal(p, &ecUnspecified); err != nil { - t.Fatalf("error unmarshaling error code %v: %v", desc.Code, err) + t.Fatalf("error unmarshaling error code %v: %v", ec, err) } if _, ok := ecUnspecified.(string); !ok { - t.Fatalf("expected a string for error code %v on unmarshal got a %T", desc.Code, ecUnspecified) + t.Fatalf("expected a string for error code %v on unmarshal got a %T", ec, ecUnspecified) } // Now, unmarshal with the error code type and ensure they are equal var ecUnmarshaled ErrorCode if err := json.Unmarshal(p, &ecUnmarshaled); err != nil { - t.Fatalf("error unmarshaling error code %v: %v", desc.Code, err) + t.Fatalf("error unmarshaling error code %v: %v", ec, err) } - if ecUnmarshaled != desc.Code { - t.Fatalf("unexpected error code during error code marshal/unmarshal: %v != %v", ecUnmarshaled, desc.Code) + if ecUnmarshaled != ec { + t.Fatalf("unexpected error code during error code marshal/unmarshal: %v != %v", ecUnmarshaled, ec) } } + } // TestErrorsManagement does a quick check of the Errors type to ensure that // members are properly pushed and marshaled. -/* +var ErrorCodeTest1 = Register("v2.errors", ErrorDescriptor{ + Value: "TEST1", + Message: "test error 1", + Description: `Just a test message #1.`, + HTTPStatusCode: http.StatusInternalServerError, +}) + +var ErrorCodeTest2 = Register("v2.errors", ErrorDescriptor{ + Value: "TEST2", + Message: "test error 2", + Description: `Just a test message #2.`, + HTTPStatusCode: http.StatusNotFound, +}) + func TestErrorsManagement(t *testing.T) { var errs Errors - errs.Push(ErrorCodeDigestInvalid) - errs.Push(ErrorCodeBlobUnknown, - map[string]digest.Digest{"digest": "sometestblobsumdoesntmatter"}) + errs = append(errs, NewError(ErrorCodeTest1)) + errs = append(errs, NewError(ErrorCodeTest2, + map[string]interface{}{"digest": "sometestblobsumdoesntmatter"})) p, err := json.Marshal(errs) @@ -70,15 +89,25 @@ func TestErrorsManagement(t *testing.T) { t.Fatalf("error marashaling errors: %v", err) } - expectedJSON := "{\"errors\":[{\"code\":\"DIGEST_INVALID\",\"message\":\"provided digest did not match uploaded content\"},{\"code\":\"BLOB_UNKNOWN\",\"message\":\"blob unknown to registry\",\"detail\":{\"digest\":\"sometestblobsumdoesntmatter\"}}]}" + expectedJSON := "[{\"code\":\"TEST1\"},{\"code\":\"TEST2\",\"detail\":{\"digest\":\"sometestblobsumdoesntmatter\"}}]" if string(p) != expectedJSON { t.Fatalf("unexpected json: %q != %q", string(p), expectedJSON) } - errs.Clear() - errs.Push(ErrorCodeUnknown) - expectedJSON = "{\"errors\":[{\"code\":\"UNKNOWN\",\"message\":\"unknown error\"}]}" + // Now test the reverse + var unmarshaled Errors + if err := json.Unmarshal(p, &unmarshaled); err != nil { + t.Fatalf("unexpected error unmarshaling error envelope: %v", err) + } + + if !reflect.DeepEqual(unmarshaled, errs) { + t.Fatalf("errors not equal after round trip:\nunmarshaled:\n%#v\n\nerrs:\n%#v", unmarshaled, errs) + } + + // Test again with a single value this time + errs = Errors{NewError(ErrorCodeUnknown)} + expectedJSON = "[{\"code\":\"UNKNOWN\"}]" p, err = json.Marshal(errs) if err != nil { @@ -88,80 +117,15 @@ func TestErrorsManagement(t *testing.T) { if string(p) != expectedJSON { t.Fatalf("unexpected json: %q != %q", string(p), expectedJSON) } -} -// TestMarshalUnmarshal ensures that api errors can round trip through json -// without losing information. -func TestMarshalUnmarshal(t *testing.T) { - - var errors Errors - - for _, testcase := range []struct { - description string - err Error - }{ - { - description: "unknown error", - err: Error{ - - Code: ErrorCodeUnknown, - Message: ErrorCodeUnknown.Descriptor().Message, - }, - }, - { - description: "unknown manifest", - err: Error{ - Code: ErrorCodeManifestUnknown, - Message: ErrorCodeManifestUnknown.Descriptor().Message, - }, - }, - { - description: "unknown manifest", - err: Error{ - Code: ErrorCodeBlobUnknown, - Message: ErrorCodeBlobUnknown.Descriptor().Message, - Detail: map[string]interface{}{"digest": "asdfqwerqwerqwerqwer"}, - }, - }, - } { - fatalf := func(format string, args ...interface{}) { - t.Fatalf(testcase.description+": "+format, args...) - } - - unexpectedErr := func(err error) { - fatalf("unexpected error: %v", err) - } - - p, err := json.Marshal(testcase.err) - if err != nil { - unexpectedErr(err) - } - - var unmarshaled Error - if err := json.Unmarshal(p, &unmarshaled); err != nil { - unexpectedErr(err) - } - - if !reflect.DeepEqual(unmarshaled, testcase.err) { - fatalf("errors not equal after round trip: %#v != %#v", unmarshaled, testcase.err) - } - - // Roll everything up into an error response envelope. - errors.PushErr(testcase.err) - } - - p, err := json.Marshal(errors) - if err != nil { - t.Fatalf("unexpected error marshaling error envelope: %v", err) - } - - var unmarshaled Errors + // Now test the reverse + unmarshaled = nil if err := json.Unmarshal(p, &unmarshaled); err != nil { t.Fatalf("unexpected error unmarshaling error envelope: %v", err) } - if !reflect.DeepEqual(unmarshaled, errors) { - t.Fatalf("errors not equal after round trip: %#v != %#v", unmarshaled, errors) + if !reflect.DeepEqual(unmarshaled, errs) { + t.Fatalf("errors not equal after round trip:\nunmarshaled:\n%#v\n\nerrs:\n%#v", unmarshaled, errs) } + } -*/ diff --git a/registry/api/v2/descriptors.go b/registry/api/v2/descriptors.go index db5a92707..d90bbb09b 100644 --- a/registry/api/v2/descriptors.go +++ b/registry/api/v2/descriptors.go @@ -172,13 +172,8 @@ const ( var APIDescriptor = struct { // RouteDescriptors provides a list of the routes available in the API. RouteDescriptors []RouteDescriptor - - // ErrorDescriptors provides a list of the error codes and their - // associated documentation and metadata. - ErrorDescriptors []errcode.ErrorDescriptor }{ RouteDescriptors: routeDescriptors, - ErrorDescriptors: errorDescriptors, } // RouteDescriptor describes a route specified by name. diff --git a/registry/api/v2/errors.go b/registry/api/v2/errors.go index 9655dba86..c12cbc1c8 100644 --- a/registry/api/v2/errors.go +++ b/registry/api/v2/errors.go @@ -6,81 +6,28 @@ import ( "github.com/docker/distribution/registry/api/errcode" ) -const ( +var ( // ErrorCodeUnsupported is returned when an operation is not supported. - ErrorCodeUnsupported = iota - - // ErrorCodeUnauthorized is returned if a request is not authorized. - ErrorCodeUnauthorized - - // ErrorCodeDigestInvalid is returned when uploading a blob if the - // provided digest does not match the blob contents. - ErrorCodeDigestInvalid - - // ErrorCodeSizeInvalid is returned when uploading a blob if the provided - // size does not match the content length. - ErrorCodeSizeInvalid - - // ErrorCodeNameInvalid is returned when the name in the manifest does not - // match the provided name. - ErrorCodeNameInvalid - - // ErrorCodeTagInvalid is returned when the tag in the manifest does not - // match the provided tag. - ErrorCodeTagInvalid - - // ErrorCodeNameUnknown when the repository name is not known. - ErrorCodeNameUnknown - - // ErrorCodeManifestUnknown returned when image manifest is unknown. - ErrorCodeManifestUnknown - - // ErrorCodeManifestInvalid returned when an image manifest is invalid, - // typically during a PUT operation. This error encompasses all errors - // encountered during manifest validation that aren't signature errors. - ErrorCodeManifestInvalid - - // ErrorCodeManifestUnverified is returned when the manifest fails - // signature verfication. - ErrorCodeManifestUnverified - - // ErrorCodeManifestBlobUnknown is returned when a manifest blob is - // unknown to the registry. - ErrorCodeManifestBlobUnknown - - // ErrorCodeBlobUnknown is returned when a blob is unknown to the - // registry. This can happen when the manifest references a nonexistent - // layer or the result is not found by a blob fetch. - ErrorCodeBlobUnknown - - // ErrorCodeBlobUploadUnknown is returned when an upload is unknown. - ErrorCodeBlobUploadUnknown - - // ErrorCodeBlobUploadInvalid is returned when an upload is invalid. - ErrorCodeBlobUploadInvalid -) - -// ErrorDescriptors provides a list of HTTP API Error codes that may be -// encountered when interacting with the registry API. -var errorDescriptors = []errcode.ErrorDescriptor{ - { - Code: ErrorCodeUnsupported, + ErrorCodeUnsupported = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "UNSUPPORTED", Message: "The operation is unsupported.", Description: `The operation was unsupported due to a missing implementation or invalid set of parameters.`, - }, - { - Code: ErrorCodeUnauthorized, + }) + + // ErrorCodeUnauthorized is returned if a request is not authorized. + ErrorCodeUnauthorized = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "UNAUTHORIZED", Message: "access to the requested resource is not authorized", Description: `The access controller denied access for the operation on a resource. Often this will be accompanied by a 401 Unauthorized response status.`, HTTPStatusCode: http.StatusForbidden, - }, - { - Code: ErrorCodeDigestInvalid, + }) + + // ErrorCodeDigestInvalid is returned when uploading a blob if the + // provided digest does not match the blob contents. + ErrorCodeDigestInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "DIGEST_INVALID", Message: "provided digest did not match uploaded content", Description: `When a blob is uploaded, the registry will check that @@ -89,50 +36,60 @@ var errorDescriptors = []errcode.ErrorDescriptor{ invalid digest string. This error may also be returned when a manifest includes an invalid layer digest.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeSizeInvalid, + }) + + // ErrorCodeSizeInvalid is returned when uploading a blob if the provided + ErrorCodeSizeInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "SIZE_INVALID", Message: "provided length did not match content length", Description: `When a layer is uploaded, the provided size will be checked against the uploaded content. If they do not match, this error will be returned.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeNameInvalid, + }) + + // ErrorCodeNameInvalid is returned when the name in the manifest does not + // match the provided name. + ErrorCodeNameInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "NAME_INVALID", Message: "invalid repository name", Description: `Invalid repository name encountered either during manifest validation or any API operation.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeTagInvalid, + }) + + // ErrorCodeTagInvalid is returned when the tag in the manifest does not + // match the provided tag. + ErrorCodeTagInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "TAG_INVALID", Message: "manifest tag did not match URI", Description: `During a manifest upload, if the tag in the manifest does not match the uri tag, this error will be returned.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeNameUnknown, + }) + + // ErrorCodeNameUnknown when the repository name is not known. + ErrorCodeNameUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "NAME_UNKNOWN", Message: "repository name not known to registry", Description: `This is returned if the name used during an operation is unknown to the registry.`, HTTPStatusCode: http.StatusNotFound, - }, - { - Code: ErrorCodeManifestUnknown, + }) + + // ErrorCodeManifestUnknown returned when image manifest is unknown. + ErrorCodeManifestUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "MANIFEST_UNKNOWN", Message: "manifest unknown", Description: `This error is returned when the manifest, identified by name and tag is unknown to the repository.`, HTTPStatusCode: http.StatusNotFound, - }, - { - Code: ErrorCodeManifestInvalid, + }) + + // ErrorCodeManifestInvalid returned when an image manifest is invalid, + // typically during a PUT operation. This error encompasses all errors + // encountered during manifest validation that aren't signature errors. + ErrorCodeManifestInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "MANIFEST_INVALID", Message: "manifest invalid", Description: `During upload, manifests undergo several checks ensuring @@ -140,25 +97,32 @@ var errorDescriptors = []errcode.ErrorDescriptor{ more specific error is included. The detail will contain information the failed validation.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeManifestUnverified, + }) + + // ErrorCodeManifestUnverified is returned when the manifest fails + // signature verfication. + ErrorCodeManifestUnverified = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "MANIFEST_UNVERIFIED", Message: "manifest failed signature verification", Description: `During manifest upload, if the manifest fails signature verification, this error will be returned.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeManifestBlobUnknown, + }) + + // ErrorCodeManifestBlobUnknown is returned when a manifest blob is + // unknown to the registry. + ErrorCodeManifestBlobUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "MANIFEST_BLOB_UNKNOWN", Message: "blob unknown to registry", Description: `This error may be returned when a manifest blob is unknown to the registry.`, HTTPStatusCode: http.StatusBadRequest, - }, - { - Code: ErrorCodeBlobUnknown, + }) + + // ErrorCodeBlobUnknown is returned when a blob is unknown to the + // registry. This can happen when the manifest references a nonexistent + // layer or the result is not found by a blob fetch. + ErrorCodeBlobUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "BLOB_UNKNOWN", Message: "blob unknown to registry", Description: `This error may be returned when a blob is unknown to the @@ -166,27 +130,23 @@ var errorDescriptors = []errcode.ErrorDescriptor{ standard get or if a manifest references an unknown layer during upload.`, HTTPStatusCode: http.StatusNotFound, - }, + }) - { - Code: ErrorCodeBlobUploadUnknown, + // ErrorCodeBlobUploadUnknown is returned when an upload is unknown. + ErrorCodeBlobUploadUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "BLOB_UPLOAD_UNKNOWN", Message: "blob upload unknown to registry", Description: `If a blob upload has been cancelled or was never started, this error code may be returned.`, HTTPStatusCode: http.StatusNotFound, - }, - { - Code: ErrorCodeBlobUploadInvalid, + }) + + // ErrorCodeBlobUploadInvalid is returned when an upload is invalid. + ErrorCodeBlobUploadInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ Value: "BLOB_UPLOAD_INVALID", Message: "blob upload invalid", Description: `The blob upload encountered an error and can no longer proceed.`, HTTPStatusCode: http.StatusNotFound, - }, -} - -// init registers our errors with the errcode system -func init() { - errcode.LoadErrors(errorDescriptors) -} + }) +) diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 12c6e2274..0ef7d4ca1 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -452,6 +452,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont if err != nil { switch err := err.(type) { case auth.Challenge: + // NOTE(duglin): // Since err.ServeHTTP will set the HTTP status code for us // we need to set the content-type here. The serveJSON // func will try to do it but it'll be too late at that point. diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index 0520cb403..d98ae4001 100644 --- a/registry/handlers/app_test.go +++ b/registry/handlers/app_test.go @@ -186,11 +186,9 @@ func TestNewApp(t *testing.T) { t.Fatalf("unexpected status code during request: %v", err) } - /* - if req.Header.Get("Content-Type") != "application/json; charset=utf-8" { - t.Fatalf("unexpected content-type: %v != %v", req.Header.Get("Content-Type"), "application/json; charset=utf-8") - } - */ + if req.Header.Get("Content-Type") != "application/json; charset=utf-8" { + t.Fatalf("unexpected content-type: %v != %v", req.Header.Get("Content-Type"), "application/json; charset=utf-8") + } expectedAuthHeader := "Bearer realm=\"realm-test\",service=\"service-test\"" if e, a := expectedAuthHeader, req.Header.Get("WWW-Authenticate"); e != a { diff --git a/registry/handlers/helpers.go b/registry/handlers/helpers.go index f4f241751..656d20667 100644 --- a/registry/handlers/helpers.go +++ b/registry/handlers/helpers.go @@ -2,9 +2,10 @@ package handlers import ( "encoding/json" - "github.com/docker/distribution/registry/api/errcode" "io" "net/http" + + "github.com/docker/distribution/registry/api/errcode" ) // serveJSON marshals v and sets the content-type header to From 441f7cac8739435e7dfe9b12c7aabdf04f7df151 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Wed, 3 Jun 2015 06:52:39 -0700 Subject: [PATCH 4/4] Round 4 Signed-off-by: Doug Davis --- cmd/registry-api-descriptor-template/main.go | 11 +- docs/spec/api.md | 24 +- registry/api/errcode/errors.go | 242 ++++++++++--------- registry/api/errcode/errors_test.go | 12 +- registry/api/errcode/register.go | 86 +++++++ registry/api/v2/errors.go | 30 +-- registry/client/blob_writer_test.go | 5 +- registry/handlers/api_test.go | 12 +- registry/handlers/app.go | 33 ++- registry/handlers/app_test.go | 8 +- registry/handlers/blob.go | 10 +- registry/handlers/blobupload.go | 50 ++-- registry/handlers/helpers.go | 11 +- registry/handlers/images.go | 26 +- registry/handlers/tags.go | 6 +- 15 files changed, 356 insertions(+), 210 deletions(-) create mode 100644 registry/api/errcode/register.go diff --git a/cmd/registry-api-descriptor-template/main.go b/cmd/registry-api-descriptor-template/main.go index dc7740363..05a1b4878 100644 --- a/cmd/registry-api-descriptor-template/main.go +++ b/cmd/registry-api-descriptor-template/main.go @@ -20,6 +20,7 @@ import ( "regexp" "text/template" + "github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/v2" ) @@ -44,7 +45,15 @@ func main() { tmpl := template.Must(template.New(filename).Funcs(funcMap).ParseFiles(path)) - if err := tmpl.Execute(os.Stdout, v2.APIDescriptor); err != nil { + data := struct { + RouteDescriptors []v2.RouteDescriptor + ErrorDescriptors []errcode.ErrorDescriptor + }{ + RouteDescriptors: v2.APIDescriptor.RouteDescriptors, + ErrorDescriptors: errcode.GetErrorCodeGroup("registry.api.v2"), + } + + if err := tmpl.Execute(os.Stdout, data); err != nil { log.Fatalln(err) } } diff --git a/docs/spec/api.md b/docs/spec/api.md index 7993b354e..4a018230b 100644 --- a/docs/spec/api.md +++ b/docs/spec/api.md @@ -734,20 +734,20 @@ The error codes encountered via the API are enumerated in the following table: |Code|Message|Description| -------|----|------|------------ - `UNKNOWN` | unknown error | Generic error returned when the error does not have an API classification. - `UNSUPPORTED` | The operation is unsupported. | The operation was unsupported due to a missing implementation or invalid set of parameters. - `UNAUTHORIZED` | access to the requested resource is not authorized | The access controller denied access for the operation on a resource. Often this will be accompanied by a 401 Unauthorized response status. - `DIGEST_INVALID` | provided digest did not match uploaded content | When a blob is uploaded, the registry will check that the content matches the digest provided by the client. The error may include a detail structure with the key "digest", including the invalid digest string. This error may also be returned when a manifest includes an invalid layer digest. - `SIZE_INVALID` | provided length did not match content length | When a layer is uploaded, the provided size will be checked against the uploaded content. If they do not match, this error will be returned. - `NAME_INVALID` | invalid repository name | Invalid repository name encountered either during manifest validation or any API operation. - `TAG_INVALID` | manifest tag did not match URI | During a manifest upload, if the tag in the manifest does not match the uri tag, this error will be returned. - `NAME_UNKNOWN` | repository name not known to registry | This is returned if the name used during an operation is unknown to the registry. - `MANIFEST_UNKNOWN` | manifest unknown | This error is returned when the manifest, identified by name and tag is unknown to the repository. - `MANIFEST_INVALID` | manifest invalid | During upload, manifests undergo several checks ensuring validity. If those checks fail, this error may be returned, unless a more specific error is included. The detail will contain information the failed validation. - `MANIFEST_UNVERIFIED` | manifest failed signature verification | During manifest upload, if the manifest fails signature verification, this error will be returned. `BLOB_UNKNOWN` | blob unknown to registry | This error may be returned when a blob is unknown to the registry in a specified repository. This can be returned with a standard get or if a manifest references an unknown layer during upload. - `BLOB_UPLOAD_UNKNOWN` | blob upload unknown to registry | If a blob upload has been cancelled or was never started, this error code may be returned. `BLOB_UPLOAD_INVALID` | blob upload invalid | The blob upload encountered an error and can no longer proceed. + `BLOB_UPLOAD_UNKNOWN` | blob upload unknown to registry | If a blob upload has been cancelled or was never started, this error code may be returned. + `DIGEST_INVALID` | provided digest did not match uploaded content | When a blob is uploaded, the registry will check that the content matches the digest provided by the client. The error may include a detail structure with the key "digest", including the invalid digest string. This error may also be returned when a manifest includes an invalid layer digest. + `MANIFEST_BLOB_UNKNOWN` | blob unknown to registry | This error may be returned when a manifest blob is unknown to the registry. + `MANIFEST_INVALID` | manifest invalid | During upload, manifests undergo several checks ensuring validity. If those checks fail, this error may be returned, unless a more specific error is included. The detail will contain information the failed validation. + `MANIFEST_UNKNOWN` | manifest unknown | This error is returned when the manifest, identified by name and tag is unknown to the repository. + `MANIFEST_UNVERIFIED` | manifest failed signature verification | During manifest upload, if the manifest fails signature verification, this error will be returned. + `NAME_INVALID` | invalid repository name | Invalid repository name encountered either during manifest validation or any API operation. + `NAME_UNKNOWN` | repository name not known to registry | This is returned if the name used during an operation is unknown to the registry. + `SIZE_INVALID` | provided length did not match content length | When a layer is uploaded, the provided size will be checked against the uploaded content. If they do not match, this error will be returned. + `TAG_INVALID` | manifest tag did not match URI | During a manifest upload, if the tag in the manifest does not match the uri tag, this error will be returned. + `UNAUTHORIZED` | access to the requested resource is not authorized | The access controller denied access for the operation on a resource. Often this will be accompanied by a 401 Unauthorized response status. + `UNSUPPORTED` | The operation is unsupported. | The operation was unsupported due to a missing implementation or invalid set of parameters. diff --git a/registry/api/errcode/errors.go b/registry/api/errcode/errors.go index 4285dedc7..cf186cfb5 100644 --- a/registry/api/errcode/errors.go +++ b/registry/api/errcode/errors.go @@ -1,106 +1,29 @@ package errcode import ( + "encoding/json" "fmt" - "net/http" "strings" - "sync" ) +// ErrorCoder is the base interface for ErrorCode and Error allowing +// users of each to just call ErrorCode to get the real ID of each +type ErrorCoder interface { + ErrorCode() ErrorCode +} + // ErrorCode represents the error type. The errors are serialized via strings // and the integer format may change and should *never* be exported. type ErrorCode int -// ErrorDescriptor provides relevant information about a given error code. -type ErrorDescriptor struct { - // Code is the error code that this descriptor describes. - Code ErrorCode - - // Value provides a unique, string key, often captilized with - // underscores, to identify the error code. This value is used as the - // keyed value when serializing api errors. - Value string - - // Message is a short, human readable decription of the error condition - // included in API responses. - Message string - - // Description provides a complete account of the errors purpose, suitable - // for use in documentation. - Description string - - // HTTPStatusCode provides the http status code that is associated with - // this error condition. - HTTPStatusCode int +// ErrorCode just returns itself +func (ec ErrorCode) ErrorCode() ErrorCode { + return ec } -var ( - errorCodeToDescriptors = map[ErrorCode]ErrorDescriptor{} - idToDescriptors = map[string]ErrorDescriptor{} - groupToDescriptors = map[string][]ErrorDescriptor{} -) - -// ErrorCodeUnknown is a generic error that can be used as a last -// resort if there is no situation-specific error message that can be used -var ErrorCodeUnknown = Register("registry.api.errcode", ErrorDescriptor{ - Value: "UNKNOWN", - Message: "unknown error", - Description: `Generic error returned when the error does not have an - API classification.`, - HTTPStatusCode: http.StatusInternalServerError, -}) - -var nextCode = 1000 -var registerLock sync.Mutex - -// Register will make the passed-in error known to the environment and -// return a new ErrorCode -func Register(group string, descriptor ErrorDescriptor) ErrorCode { - registerLock.Lock() - defer registerLock.Unlock() - code := ErrorCode(nextCode) - - descriptor.Code = code - - if _, ok := idToDescriptors[descriptor.Value]; ok { - panic(fmt.Sprintf("ErrorValue %s is already registered", descriptor.Value)) - } - if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { - panic(fmt.Sprintf("ErrorCode %d is already registered", descriptor.Code)) - } - - groupToDescriptors[group] = append(groupToDescriptors[group], descriptor) - errorCodeToDescriptors[code] = descriptor - idToDescriptors[descriptor.Value] = descriptor - - nextCode++ - return code -} - -// ParseErrorCode returns the value by the string error code. -// `ErrorCodeUnknown` will be returned if the error is not known. -func ParseErrorCode(value string) ErrorCode { - ed, ok := idToDescriptors[value] - if ok { - return ed.Code - } - - return ErrorCodeUnknown -} - -// GetGroupNames returns the list of Error group names that are registered -func GetGroupNames() []string { - keys := []string{} - - for k := range groupToDescriptors { - keys = append(keys, k) - } - return keys -} - -// GetErrorCodeGroup returns the named group of error descriptors -func GetErrorCodeGroup(name string) []ErrorDescriptor { - return groupToDescriptors[name] +// Error returns the ID/Value +func (ec ErrorCode) Error() string { + return ec.Descriptor().Value } // Descriptor returns the descriptor for the error code. @@ -143,12 +66,30 @@ func (ec *ErrorCode) UnmarshalText(text []byte) error { return nil } +// WithDetail creates a new Error struct based on the passed-in info and +// set the Detail property appropriately +func (ec ErrorCode) WithDetail(detail interface{}) Error { + if err, ok := detail.(error); ok { + detail = err.Error() + } + + return Error{ + Code: ec, + Detail: detail, + } +} + // Error provides a wrapper around ErrorCode with extra Details provided. type Error struct { Code ErrorCode `json:"code"` Detail interface{} `json:"detail,omitempty"` } +// ErrorCode returns the ID/Value of this Error +func (e Error) ErrorCode() ErrorCode { + return e.Code +} + // Error returns a human readable representation of the error. func (e Error) Error() string { return fmt.Sprintf("%s: %s", @@ -161,30 +102,43 @@ func (e Error) Message() string { return e.Code.Message() } +// ErrorDescriptor provides relevant information about a given error code. +type ErrorDescriptor struct { + // Code is the error code that this descriptor describes. + Code ErrorCode + + // Value provides a unique, string key, often captilized with + // underscores, to identify the error code. This value is used as the + // keyed value when serializing api errors. + Value string + + // Message is a short, human readable decription of the error condition + // included in API responses. + Message string + + // Description provides a complete account of the errors purpose, suitable + // for use in documentation. + Description string + + // HTTPStatusCode provides the http status code that is associated with + // this error condition. + HTTPStatusCode int +} + +// ParseErrorCode returns the value by the string error code. +// `ErrorCodeUnknown` will be returned if the error is not known. +func ParseErrorCode(value string) ErrorCode { + ed, ok := idToDescriptors[value] + if ok { + return ed.Code + } + + return ErrorCodeUnknown +} + // Errors provides the envelope for multiple errors and a few sugar methods // for use within the application. -type Errors []Error - -// NewError creates a new Error struct based on the passed-in info -func NewError(code ErrorCode, details ...interface{}) Error { - if len(details) > 1 { - panic("please specify zero or one detail items for this error") - } - - var detail interface{} - if len(details) > 0 { - detail = details[0] - } - - if err, ok := detail.(error); ok { - detail = err.Error() - } - - return Error{ - Code: code, - Detail: detail, - } -} +type Errors []error func (errs Errors) Error() string { switch len(errs) { @@ -205,3 +159,67 @@ func (errs Errors) Error() string { func (errs Errors) Len() int { return len(errs) } + +// jsonError extends Error with 'Message' so that we can include the +// error text, just in case the receiver of the JSON doesn't have this +// particular ErrorCode registered +type jsonError struct { + Code ErrorCode `json:"code"` + Message string `json:"message"` + Detail interface{} `json:"detail,omitempty"` +} + +// MarshalJSON converts slice of error, ErrorCode or Error into a +// slice of Error - then serializes +func (errs Errors) MarshalJSON() ([]byte, error) { + var tmpErrs []jsonError + + for _, daErr := range errs { + var err Error + + switch daErr.(type) { + case ErrorCode: + err = daErr.(ErrorCode).WithDetail(nil) + case Error: + err = daErr.(Error) + default: + err = ErrorCodeUnknown.WithDetail(daErr) + + } + + tmpErrs = append(tmpErrs, jsonError{ + Code: err.Code, + Message: err.Message(), + Detail: err.Detail, + }) + } + + return json.Marshal(tmpErrs) +} + +// UnmarshalJSON deserializes []Error and then converts it into slice of +// Error or ErrorCode +func (errs *Errors) UnmarshalJSON(data []byte) error { + var tmpErrs []jsonError + + if err := json.Unmarshal(data, &tmpErrs); err != nil { + return err + } + + var newErrs Errors + for _, daErr := range tmpErrs { + if daErr.Detail == nil { + // Error's w/o details get converted to ErrorCode + newErrs = append(newErrs, daErr.Code) + } else { + // Error's w/ details are untouched + newErrs = append(newErrs, Error{ + Code: daErr.Code, + Detail: daErr.Detail, + }) + } + } + + *errs = newErrs + return nil +} diff --git a/registry/api/errcode/errors_test.go b/registry/api/errcode/errors_test.go index aaf0d73b7..d89c02537 100644 --- a/registry/api/errcode/errors_test.go +++ b/registry/api/errcode/errors_test.go @@ -79,8 +79,8 @@ var ErrorCodeTest2 = Register("v2.errors", ErrorDescriptor{ func TestErrorsManagement(t *testing.T) { var errs Errors - errs = append(errs, NewError(ErrorCodeTest1)) - errs = append(errs, NewError(ErrorCodeTest2, + errs = append(errs, ErrorCodeTest1) + errs = append(errs, ErrorCodeTest2.WithDetail( map[string]interface{}{"digest": "sometestblobsumdoesntmatter"})) p, err := json.Marshal(errs) @@ -89,10 +89,10 @@ func TestErrorsManagement(t *testing.T) { t.Fatalf("error marashaling errors: %v", err) } - expectedJSON := "[{\"code\":\"TEST1\"},{\"code\":\"TEST2\",\"detail\":{\"digest\":\"sometestblobsumdoesntmatter\"}}]" + expectedJSON := "[{\"code\":\"TEST1\",\"message\":\"test error 1\"},{\"code\":\"TEST2\",\"message\":\"test error 2\",\"detail\":{\"digest\":\"sometestblobsumdoesntmatter\"}}]" if string(p) != expectedJSON { - t.Fatalf("unexpected json: %q != %q", string(p), expectedJSON) + t.Fatalf("unexpected json:\ngot:\n%q\n\nexpected:\n%q", string(p), expectedJSON) } // Now test the reverse @@ -106,8 +106,8 @@ func TestErrorsManagement(t *testing.T) { } // Test again with a single value this time - errs = Errors{NewError(ErrorCodeUnknown)} - expectedJSON = "[{\"code\":\"UNKNOWN\"}]" + errs = Errors{ErrorCodeUnknown} + expectedJSON = "[{\"code\":\"UNKNOWN\",\"message\":\"unknown error\"}]" p, err = json.Marshal(errs) if err != nil { diff --git a/registry/api/errcode/register.go b/registry/api/errcode/register.go new file mode 100644 index 000000000..42f911b31 --- /dev/null +++ b/registry/api/errcode/register.go @@ -0,0 +1,86 @@ +package errcode + +import ( + "fmt" + "net/http" + "sort" + "sync" +) + +var ( + errorCodeToDescriptors = map[ErrorCode]ErrorDescriptor{} + idToDescriptors = map[string]ErrorDescriptor{} + groupToDescriptors = map[string][]ErrorDescriptor{} +) + +// ErrorCodeUnknown is a generic error that can be used as a last +// resort if there is no situation-specific error message that can be used +var ErrorCodeUnknown = Register("errcode", ErrorDescriptor{ + Value: "UNKNOWN", + Message: "unknown error", + Description: `Generic error returned when the error does not have an + API classification.`, + HTTPStatusCode: http.StatusInternalServerError, +}) + +var nextCode = 1000 +var registerLock sync.Mutex + +// Register will make the passed-in error known to the environment and +// return a new ErrorCode +func Register(group string, descriptor ErrorDescriptor) ErrorCode { + registerLock.Lock() + defer registerLock.Unlock() + + descriptor.Code = ErrorCode(nextCode) + + if _, ok := idToDescriptors[descriptor.Value]; ok { + panic(fmt.Sprintf("ErrorValue %q is already registered", descriptor.Value)) + } + if _, ok := errorCodeToDescriptors[descriptor.Code]; ok { + panic(fmt.Sprintf("ErrorCode %v is already registered", descriptor.Code)) + } + + groupToDescriptors[group] = append(groupToDescriptors[group], descriptor) + errorCodeToDescriptors[descriptor.Code] = descriptor + idToDescriptors[descriptor.Value] = descriptor + + nextCode++ + return descriptor.Code +} + +type byValue []ErrorDescriptor + +func (a byValue) Len() int { return len(a) } +func (a byValue) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byValue) Less(i, j int) bool { return a[i].Value < a[j].Value } + +// GetGroupNames returns the list of Error group names that are registered +func GetGroupNames() []string { + keys := []string{} + + for k := range groupToDescriptors { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +// GetErrorCodeGroup returns the named group of error descriptors +func GetErrorCodeGroup(name string) []ErrorDescriptor { + desc := groupToDescriptors[name] + sort.Sort(byValue(desc)) + return desc +} + +// GetErrorAllDescriptors returns a slice of all ErrorDescriptors that are +// registered, irrespective of what group they're in +func GetErrorAllDescriptors() []ErrorDescriptor { + result := []ErrorDescriptor{} + + for _, group := range GetGroupNames() { + result = append(result, GetErrorCodeGroup(group)...) + } + sort.Sort(byValue(result)) + return result +} diff --git a/registry/api/v2/errors.go b/registry/api/v2/errors.go index c12cbc1c8..14684560a 100644 --- a/registry/api/v2/errors.go +++ b/registry/api/v2/errors.go @@ -6,9 +6,11 @@ import ( "github.com/docker/distribution/registry/api/errcode" ) +const errGroup = "registry.api.v2" + var ( // ErrorCodeUnsupported is returned when an operation is not supported. - ErrorCodeUnsupported = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeUnsupported = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "UNSUPPORTED", Message: "The operation is unsupported.", Description: `The operation was unsupported due to a missing @@ -16,7 +18,7 @@ var ( }) // ErrorCodeUnauthorized is returned if a request is not authorized. - ErrorCodeUnauthorized = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeUnauthorized = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "UNAUTHORIZED", Message: "access to the requested resource is not authorized", Description: `The access controller denied access for the operation on @@ -27,7 +29,7 @@ var ( // ErrorCodeDigestInvalid is returned when uploading a blob if the // provided digest does not match the blob contents. - ErrorCodeDigestInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeDigestInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "DIGEST_INVALID", Message: "provided digest did not match uploaded content", Description: `When a blob is uploaded, the registry will check that @@ -39,7 +41,7 @@ var ( }) // ErrorCodeSizeInvalid is returned when uploading a blob if the provided - ErrorCodeSizeInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeSizeInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "SIZE_INVALID", Message: "provided length did not match content length", Description: `When a layer is uploaded, the provided size will be @@ -50,7 +52,7 @@ var ( // ErrorCodeNameInvalid is returned when the name in the manifest does not // match the provided name. - ErrorCodeNameInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeNameInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "NAME_INVALID", Message: "invalid repository name", Description: `Invalid repository name encountered either during @@ -60,7 +62,7 @@ var ( // ErrorCodeTagInvalid is returned when the tag in the manifest does not // match the provided tag. - ErrorCodeTagInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeTagInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "TAG_INVALID", Message: "manifest tag did not match URI", Description: `During a manifest upload, if the tag in the manifest @@ -69,7 +71,7 @@ var ( }) // ErrorCodeNameUnknown when the repository name is not known. - ErrorCodeNameUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeNameUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "NAME_UNKNOWN", Message: "repository name not known to registry", Description: `This is returned if the name used during an operation is @@ -78,7 +80,7 @@ var ( }) // ErrorCodeManifestUnknown returned when image manifest is unknown. - ErrorCodeManifestUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_UNKNOWN", Message: "manifest unknown", Description: `This error is returned when the manifest, identified by @@ -89,7 +91,7 @@ var ( // ErrorCodeManifestInvalid returned when an image manifest is invalid, // typically during a PUT operation. This error encompasses all errors // encountered during manifest validation that aren't signature errors. - ErrorCodeManifestInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_INVALID", Message: "manifest invalid", Description: `During upload, manifests undergo several checks ensuring @@ -101,7 +103,7 @@ var ( // ErrorCodeManifestUnverified is returned when the manifest fails // signature verfication. - ErrorCodeManifestUnverified = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestUnverified = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_UNVERIFIED", Message: "manifest failed signature verification", Description: `During manifest upload, if the manifest fails signature @@ -111,7 +113,7 @@ var ( // ErrorCodeManifestBlobUnknown is returned when a manifest blob is // unknown to the registry. - ErrorCodeManifestBlobUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeManifestBlobUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "MANIFEST_BLOB_UNKNOWN", Message: "blob unknown to registry", Description: `This error may be returned when a manifest blob is @@ -122,7 +124,7 @@ var ( // ErrorCodeBlobUnknown is returned when a blob is unknown to the // registry. This can happen when the manifest references a nonexistent // layer or the result is not found by a blob fetch. - ErrorCodeBlobUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeBlobUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "BLOB_UNKNOWN", Message: "blob unknown to registry", Description: `This error may be returned when a blob is unknown to the @@ -133,7 +135,7 @@ var ( }) // ErrorCodeBlobUploadUnknown is returned when an upload is unknown. - ErrorCodeBlobUploadUnknown = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeBlobUploadUnknown = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "BLOB_UPLOAD_UNKNOWN", Message: "blob upload unknown to registry", Description: `If a blob upload has been cancelled or was never @@ -142,7 +144,7 @@ var ( }) // ErrorCodeBlobUploadInvalid is returned when an upload is invalid. - ErrorCodeBlobUploadInvalid = errcode.Register("registry.api.v2", errcode.ErrorDescriptor{ + ErrorCodeBlobUploadInvalid = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "BLOB_UPLOAD_INVALID", Message: "blob upload invalid", Description: `The blob upload encountered an error and can no diff --git a/registry/client/blob_writer_test.go b/registry/client/blob_writer_test.go index 74545b065..eeb9f53d3 100644 --- a/registry/client/blob_writer_test.go +++ b/registry/client/blob_writer_test.go @@ -164,7 +164,10 @@ func TestUploadReadFrom(t *testing.T) { } else if len(uploadErr) != 1 { t.Fatalf("Unexpected number of errors: %d, expected 1", len(uploadErr)) } else { - v2Err := uploadErr[0] + v2Err, ok := uploadErr[0].(errcode.Error) + if !ok { + t.Fatalf("Not an 'Error' type: %#v", uploadErr[0]) + } if v2Err.Code != v2.ErrorCodeBlobUploadInvalid { t.Fatalf("Unexpected error code: %s, expected %d", v2Err.Code.String(), v2.ErrorCodeBlobUploadInvalid) } diff --git a/registry/handlers/api_test.go b/registry/handlers/api_test.go index 146fcf4c9..9952d68ef 100644 --- a/registry/handlers/api_test.go +++ b/registry/handlers/api_test.go @@ -780,11 +780,15 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error counts[code] = 0 } - for _, err := range errs { - if _, ok := expected[err.Code]; !ok { - t.Fatalf("unexpected error code %v encountered during %s: %s ", err.Code, msg, string(p)) + for _, e := range errs { + err, ok := e.(errcode.ErrorCoder) + if !ok { + t.Fatalf("not an ErrorCoder: %#v", e) } - counts[err.Code]++ + if _, ok := expected[err.ErrorCode()]; !ok { + t.Fatalf("unexpected error code %v encountered during %s: %s ", err.ErrorCode(), msg, string(p)) + } + counts[err.ErrorCode()]++ } // Ensure that counts of expected errors were all non-zero diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 0ef7d4ca1..83b231afb 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -346,9 +346,9 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - context.Errors = append(context.Errors, errcode.NewError(v2.ErrorCodeNameUnknown, err)) + context.Errors = append(context.Errors, v2.ErrorCodeNameUnknown.WithDetail(err)) case distribution.ErrRepositoryNameInvalid: - context.Errors = append(context.Errors, errcode.NewError(v2.ErrorCodeNameInvalid, err)) + context.Errors = append(context.Errors, v2.ErrorCodeNameInvalid.WithDetail(err)) } serveJSON(w, context.Errors) @@ -363,7 +363,7 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { context.Repository, err = applyRepoMiddleware(context.Repository, app.Config.Middleware["repository"]) if err != nil { ctxu.GetLogger(context).Errorf("error initializing repository middleware: %v", err) - context.Errors = append(context.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + context.Errors = append(context.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) serveJSON(w, context.Errors) return @@ -383,10 +383,25 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler { } func (app *App) logError(context context.Context, errors errcode.Errors) { - for _, e := range errors { - c := ctxu.WithValue(context, "err.code", e.Code) - c = ctxu.WithValue(c, "err.message", e.Code.Message()) - c = ctxu.WithValue(c, "err.detail", e.Detail) + for _, e1 := range errors { + var c ctxu.Context + + switch e1.(type) { + case errcode.Error: + e, _ := e1.(errcode.Error) + c = ctxu.WithValue(context, "err.code", e.Code) + c = ctxu.WithValue(c, "err.message", e.Code.Message()) + c = ctxu.WithValue(c, "err.detail", e.Detail) + case errcode.ErrorCode: + e, _ := e1.(errcode.ErrorCode) + c = ctxu.WithValue(context, "err.code", e) + c = ctxu.WithValue(c, "err.message", e.Message()) + default: + // just normal go 'error' + c = ctxu.WithValue(context, "err.code", errcode.ErrorCodeUnknown) + c = ctxu.WithValue(c, "err.message", e1.Error()) + } + c = ctxu.WithLogger(c, ctxu.GetLogger(c, "err.code", "err.message", @@ -441,7 +456,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont // proceed. var errs errcode.Errors - errs = append(errs, errcode.NewError(v2.ErrorCodeUnauthorized)) + errs = append(errs, v2.ErrorCodeUnauthorized) serveJSON(w, errs) return fmt.Errorf("forbidden: no repository name") @@ -465,7 +480,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont err.ServeHTTP(w, r) var errs errcode.Errors - errs = append(errs, errcode.NewError(v2.ErrorCodeUnauthorized, accessRecords)) + errs = append(errs, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)) serveJSON(w, errs) default: // This condition is a potential security problem either in diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go index d98ae4001..98ecaefd5 100644 --- a/registry/handlers/app_test.go +++ b/registry/handlers/app_test.go @@ -201,8 +201,12 @@ func TestNewApp(t *testing.T) { t.Fatalf("error decoding error response: %v", err) } - if errs[0].Code != v2.ErrorCodeUnauthorized { - t.Fatalf("unexpected error code: %v != %v", errs[0].Code, v2.ErrorCodeUnauthorized) + err2, ok := errs[0].(errcode.ErrorCoder) + if !ok { + t.Fatalf("not an ErrorCoder: %#v", errs[0]) + } + if err2.ErrorCode() != v2.ErrorCodeUnauthorized { + t.Fatalf("unexpected error code: %v != %v", err2.ErrorCode(), v2.ErrorCodeUnauthorized) } } diff --git a/registry/handlers/blob.go b/registry/handlers/blob.go index fa9f576aa..e33bd3c01 100644 --- a/registry/handlers/blob.go +++ b/registry/handlers/blob.go @@ -18,12 +18,12 @@ func blobDispatcher(ctx *Context, r *http.Request) http.Handler { if err == errDigestNotAvailable { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors = append(ctx.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + ctx.Errors = append(ctx.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx.Errors = append(ctx.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + ctx.Errors = append(ctx.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) }) } @@ -53,16 +53,16 @@ func (bh *blobHandler) GetBlob(w http.ResponseWriter, r *http.Request) { desc, err := blobs.Stat(bh, bh.Digest) if err != nil { if err == distribution.ErrBlobUnknown { - bh.Errors = append(bh.Errors, errcode.NewError(v2.ErrorCodeBlobUnknown, bh.Digest)) + bh.Errors = append(bh.Errors, v2.ErrorCodeBlobUnknown.WithDetail(bh.Digest)) } else { - bh.Errors = append(bh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } return } if err := blobs.ServeBlob(bh, w, r, desc.Digest); err != nil { context.GetLogger(bh).Debugf("unexpected error getting blob HTTP handler: %v", err) - bh.Errors = append(bh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } } diff --git a/registry/handlers/blobupload.go b/registry/handlers/blobupload.go index 7e8c39622..8dc417baa 100644 --- a/registry/handlers/blobupload.go +++ b/registry/handlers/blobupload.go @@ -37,7 +37,7 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if err != nil { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("error resolving upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) }) } buh.State = state @@ -45,14 +45,14 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { if state.Name != ctx.Repository.Name() { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched repository name in upload state: %q != %q", state.Name, buh.Repository.Name()) - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) }) } if state.UUID != buh.UUID { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctxu.GetLogger(ctx).Infof("mismatched uuid in upload state: %q != %q", state.UUID, buh.UUID) - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) }) } @@ -62,12 +62,12 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { ctxu.GetLogger(ctx).Errorf("error resolving upload: %v", err) if err == distribution.ErrBlobUploadUnknown { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown.WithDetail(err)) }) } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) }) } buh.Upload = upload @@ -81,14 +81,14 @@ func blobUploadDispatcher(ctx *Context, r *http.Request) http.Handler { defer upload.Close() ctxu.GetLogger(ctx).Infof("error seeking blob upload: %v", err) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) upload.Cancel(buh) }) } else if nn != buh.State.Offset { defer upload.Close() ctxu.GetLogger(ctx).Infof("seek to wrong offest: %d != %d", nn, buh.State.Offset) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) upload.Cancel(buh) }) } @@ -119,7 +119,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req blobs := buh.Repository.Blobs(buh) upload, err := blobs.Create(buh) if err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -127,7 +127,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req defer buh.Upload.Close() if err := buh.blobUploadResponse(w, r, true); err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -138,7 +138,7 @@ func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Req // GetUploadStatus returns the status of a given upload, identified by id. func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } @@ -146,7 +146,7 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // resumable upload is supported. This will enable returning a non-zero // range for clients to begin uploading at an offset. if err := buh.blobUploadResponse(w, r, true); err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -157,13 +157,13 @@ func (buh *blobUploadHandler) GetUploadStatus(w http.ResponseWriter, r *http.Req // PatchBlobData writes data to an upload. func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } ct := r.Header.Get("Content-Type") if ct != "" && ct != "application/octet-stream" { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, fmt.Errorf("Bad Content-Type"))) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(fmt.Errorf("Bad Content-Type"))) // TODO(dmcgowan): encode error return } @@ -173,12 +173,12 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // Copy the data if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } if err := buh.blobUploadResponse(w, r, false); err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -192,7 +192,7 @@ func (buh *blobUploadHandler) PatchBlobData(w http.ResponseWriter, r *http.Reque // url of the blob. func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } @@ -200,21 +200,21 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if dgstStr == "" { // no digest? return error, but allow retry. - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, "digest missing")) + buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail("digest missing")) return } dgst, err := digest.ParseDigest(dgstStr) if err != nil { // no digest? return error, but allow retry. - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, "digest parsing failed")) + buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail("digest parsing failed")) return } // Read in the data, if any. if _, err := io.Copy(buh.Upload, r.Body); err != nil { ctxu.GetLogger(buh).Errorf("unknown error copying into upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -229,14 +229,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht if err != nil { switch err := err.(type) { case distribution.ErrBlobInvalidDigest: - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) default: switch err { case distribution.ErrBlobInvalidLength, distribution.ErrBlobDigestUnsupported: - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadInvalid, err)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err)) default: ctxu.GetLogger(buh).Errorf("unknown error completing upload: %#v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } } @@ -253,7 +253,7 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // Build our canonical blob url blobURL, err := buh.urlBuilder.BuildBlobURL(buh.Repository.Name(), desc.Digest) if err != nil { - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } @@ -266,14 +266,14 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht // CancelBlobUpload cancels an in-progress upload of a blob. func (buh *blobUploadHandler) CancelBlobUpload(w http.ResponseWriter, r *http.Request) { if buh.Upload == nil { - buh.Errors = append(buh.Errors, errcode.NewError(v2.ErrorCodeBlobUploadUnknown)) + buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadUnknown) return } w.Header().Set("Docker-Upload-UUID", buh.UUID) if err := buh.Upload.Cancel(buh); err != nil { ctxu.GetLogger(buh).Errorf("error encountered canceling upload: %v", err) - buh.Errors = append(buh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } w.WriteHeader(http.StatusNoContent) diff --git a/registry/handlers/helpers.go b/registry/handlers/helpers.go index 656d20667..c72c57840 100644 --- a/registry/handlers/helpers.go +++ b/registry/handlers/helpers.go @@ -16,9 +16,14 @@ func serveJSON(w http.ResponseWriter, v interface{}) error { sc := http.StatusInternalServerError if errs, ok := v.(errcode.Errors); ok && len(errs) > 0 { - sc = errs[0].Code.Descriptor().HTTPStatusCode - if sc == 0 { - sc = http.StatusInternalServerError + if err, ok := errs[0].(errcode.ErrorCoder); ok { + if sc2 := err.ErrorCode().Descriptor().HTTPStatusCode; sc2 != 0 { + sc = sc2 + } + } + } else if err, ok := v.(errcode.ErrorCoder); ok { + if sc2 := err.ErrorCode().Descriptor().HTTPStatusCode; sc2 != 0 { + sc = sc2 } } diff --git a/registry/handlers/images.go b/registry/handlers/images.go index 9d025c787..41fbabc43 100644 --- a/registry/handlers/images.go +++ b/registry/handlers/images.go @@ -64,7 +64,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http } if err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestUnknown, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err)) return } @@ -72,7 +72,7 @@ func (imh *imageManifestHandler) GetImageManifest(w http.ResponseWriter, r *http if imh.Digest == "" { dgst, err := digestManifest(imh, sm) if err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) return } @@ -93,13 +93,13 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http var manifest manifest.SignedManifest if err := dec.Decode(&manifest); err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestInvalid, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err)) return } dgst, err := digestManifest(imh, &manifest) if err != nil { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid, err)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err)) return } @@ -107,7 +107,7 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http if imh.Tag != "" { if manifest.Tag != imh.Tag { ctxu.GetLogger(imh).Errorf("invalid tag on manifest payload: %q != %q", manifest.Tag, imh.Tag) - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeTagInvalid)) + imh.Errors = append(imh.Errors, v2.ErrorCodeTagInvalid) return } @@ -115,11 +115,11 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http } else if imh.Digest != "" { if dgst != imh.Digest { ctxu.GetLogger(imh).Errorf("payload digest does match: %q != %q", dgst, imh.Digest) - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid) return } } else { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeTagInvalid, "no tag or digest specified")) + imh.Errors = append(imh.Errors, v2.ErrorCodeTagInvalid.WithDetail("no tag or digest specified")) return } @@ -131,19 +131,19 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http for _, verificationError := range err { switch verificationError := verificationError.(type) { case distribution.ErrManifestBlobUnknown: - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeBlobUnknown, verificationError.Digest)) + imh.Errors = append(imh.Errors, v2.ErrorCodeBlobUnknown.WithDetail(verificationError.Digest)) case distribution.ErrManifestUnverified: - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeManifestUnverified)) + imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnverified) default: if verificationError == digest.ErrDigestInvalidFormat { - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeDigestInvalid)) + imh.Errors = append(imh.Errors, v2.ErrorCodeDigestInvalid) } else { - imh.Errors = append(imh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, verificationError)) + imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown, verificationError) } } } default: - imh.Errors = append(imh.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } return @@ -172,7 +172,7 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h // tag index entries a serious problem in eventually consistent storage. // Once we work out schema version 2, the full deletion system will be // worked out and we can add support back. - imh.Errors = append(imh.Errors, errcode.NewError(v2.ErrorCodeUnsupported)) + imh.Errors = append(imh.Errors, v2.ErrorCodeUnsupported) } // digestManifest takes a digest of the given manifest. This belongs somewhere diff --git a/registry/handlers/tags.go b/registry/handlers/tags.go index e1846cf96..00f9760ed 100644 --- a/registry/handlers/tags.go +++ b/registry/handlers/tags.go @@ -40,9 +40,9 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { if err != nil { switch err := err.(type) { case distribution.ErrRepositoryUnknown: - th.Errors = append(th.Errors, errcode.NewError(v2.ErrorCodeNameUnknown, map[string]string{"name": th.Repository.Name()})) + th.Errors = append(th.Errors, v2.ErrorCodeNameUnknown.WithDetail(map[string]string{"name": th.Repository.Name()})) default: - th.Errors = append(th.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + th.Errors = append(th.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) } return } @@ -54,7 +54,7 @@ func (th *tagsHandler) GetTags(w http.ResponseWriter, r *http.Request) { Name: th.Repository.Name(), Tags: tags, }); err != nil { - th.Errors = append(th.Errors, errcode.NewError(errcode.ErrorCodeUnknown, err)) + th.Errors = append(th.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) return } }