pass thru upstream error code (#4917)

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
 	}
 }

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)}
+	}
+}

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)
+			}
+		})
+	}
+}

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
 	}