diff --git a/registry/handlers/blob.go b/registry/handlers/blob.go index b4979782f..fa96d40a9 100644 --- a/registry/handlers/blob.go +++ b/registry/handlers/blob.go @@ -60,14 +60,14 @@ func (bh *blobHandler) GetBlob(w http.ResponseWriter, r *http.Request) { if err == distribution.ErrBlobUnknown { bh.Errors = append(bh.Errors, errcode.ErrorCodeBlobUnknown.WithDetail(bh.Digest)) } else { - bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) + bh.Errors = append(bh.Errors, toErrcodeErrors(err)...) } return } if err := blobs.ServeBlob(bh, w, r, desc.Digest); err != nil { dcontext.GetLogger(bh).Debugf("unexpected error getting blob HTTP handler: %v", err) - bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) + bh.Errors = append(bh.Errors, toErrcodeErrors(err)...) return } } diff --git a/registry/handlers/helpers.go b/registry/handlers/helpers.go index b1bdea34d..64cd4f4cb 100644 --- a/registry/handlers/helpers.go +++ b/registry/handlers/helpers.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/distribution/distribution/v3/internal/dcontext" + "github.com/distribution/distribution/v3/registry/api/errcode" ) // closeResources closes all the provided resources after running the target @@ -83,3 +84,19 @@ func parseContentRange(cr string) (start int64, end int64, err error) { } return start, end, nil } + +// toErrcodeErrors converts err into the errcode.Errors reported to the client. +// When err already carries one or more registry error codes (for eg 401/403), +// those codes are preserved so the client receives the correct status instead +// of a generic internal server error. Errors that do not carry a code are reported +// as ErrorCodeUnknown, preserving the previous behaviour. +func toErrcodeErrors(err error) errcode.Errors { + switch err := err.(type) { + case errcode.Errors: + return err + case errcode.Error: + return errcode.Errors{err} + default: + return errcode.Errors{errcode.ErrorCodeUnknown.WithDetail(err)} + } +} diff --git a/registry/handlers/helpers_test.go b/registry/handlers/helpers_test.go new file mode 100644 index 000000000..cb6790267 --- /dev/null +++ b/registry/handlers/helpers_test.go @@ -0,0 +1,54 @@ +package handlers + +import ( + "errors" + "net/http" + "testing" + + "github.com/distribution/distribution/v3/registry/api/errcode" +) + +func TestErrcodeErrorsFor(t *testing.T) { + for _, tc := range []struct { + name string + err error + wantCode errcode.ErrorCode + wantHTTPStatus int + }{ + { + name: "upstream errcode.Errors are preserved", + err: errcode.Errors{errcode.ErrorCodeDenied.WithMessage("requested access to the resource is denied"), errcode.ErrorCodeUnauthorized.WithMessage("authentication required")}, + wantCode: errcode.ErrorCodeDenied, + wantHTTPStatus: http.StatusForbidden, + }, + { + name: "single coded error is preserved", + err: errcode.ErrorCodeUnauthorized.WithMessage("authentication required"), + wantCode: errcode.ErrorCodeUnauthorized, + wantHTTPStatus: http.StatusUnauthorized, + }, + { + name: "uncoded error becomes unknown", + err: errors.New("boom"), + wantCode: errcode.ErrorCodeUnknown, + wantHTTPStatus: http.StatusInternalServerError, + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := toErrcodeErrors(tc.err) + if len(got) == 0 { + t.Fatalf("errcodeErrorsFor(%v) returned no errors", tc.err) + } + coder, ok := got[0].(errcode.ErrorCoder) + if !ok { + t.Fatalf("first error %#v does not implement errcode.ErrorCoder", got[0]) + } + if coder.ErrorCode() != tc.wantCode { + t.Errorf("code = %v, want %v", coder.ErrorCode(), tc.wantCode) + } + if sc := coder.ErrorCode().Descriptor().HTTPStatusCode; sc != tc.wantHTTPStatus { + t.Errorf("HTTP status = %d, want %d", sc, tc.wantHTTPStatus) + } + }) + } +} diff --git a/registry/handlers/manifests.go b/registry/handlers/manifests.go index 91688ba42..f47667f78 100644 --- a/registry/handlers/manifests.go +++ b/registry/handlers/manifests.go @@ -124,7 +124,7 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) if _, ok := err.(distribution.ErrTagUnknown); ok { imh.Errors = append(imh.Errors, errcode.ErrorCodeManifestUnknown.WithDetail(err)) } else { - imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) + imh.Errors = append(imh.Errors, toErrcodeErrors(err)...) } return } @@ -145,7 +145,7 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request) if _, ok := err.(distribution.ErrManifestUnknownRevision); ok { imh.Errors = append(imh.Errors, errcode.ErrorCodeManifestUnknown.WithDetail(err)) } else { - imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err)) + imh.Errors = append(imh.Errors, toErrcodeErrors(err)...) } return }