diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index 49fda123d0..ad0af92ced 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -350,7 +350,7 @@ responses: description: The ID of the corresponding request for the response type: string schema: - $ref: '#/definitions/Error' + $ref: '#/definitions/Errors' '401': description: Unauthorized headers: @@ -358,7 +358,7 @@ responses: description: The ID of the corresponding request for the response type: string schema: - $ref: '#/definitions/Error' + $ref: '#/definitions/Errors' '403': description: Forbidden headers: @@ -366,7 +366,7 @@ responses: description: The ID of the corresponding request for the response type: string schema: - $ref: '#/definitions/Error' + $ref: '#/definitions/Errors' '404': description: Not found headers: @@ -374,7 +374,7 @@ responses: description: The ID of the corresponding request for the response type: string schema: - $ref: '#/definitions/Error' + $ref: '#/definitions/Errors' '409': description: Conflict headers: @@ -382,7 +382,7 @@ responses: description: The ID of the corresponding request for the response type: string schema: - $ref: '#/definitions/Error' + $ref: '#/definitions/Errors' '500': description: Internal server error headers: @@ -390,8 +390,13 @@ responses: description: The ID of the corresponding request for the response type: string schema: - $ref: '#/definitions/Error' + $ref: '#/definitions/Errors' definitions: + Errors: + description: The error array that describe the errors got during the handling of request + type: array + items: + $ref: '#/definitions/Error' Error: description: a model for all the error response coming from harbor type: object @@ -402,11 +407,6 @@ definitions: message: type: string description: The error message - x-go-type: - import: - package: "github.com/goharbor/harbor/src/internal/error" - alias: "ierrors" - type: "Error" Artifact: type: object properties: diff --git a/src/common/api/base.go b/src/common/api/base.go index 7b55ad2409..1547a090b5 100644 --- a/src/common/api/base.go +++ b/src/common/api/base.go @@ -262,6 +262,5 @@ func (b *BaseAPI) SendStatusServiceUnavailableError(err error) { // ] // } func (b *BaseAPI) SendError(err error) { - statusCode, payload := serror.APIError(err) - b.RenderError(statusCode, payload) + serror.SendError(b.Ctx.ResponseWriter, err) } diff --git a/src/server/error/error.go b/src/server/error/error.go index 851ad5a01a..99c77756e5 100644 --- a/src/server/error/error.go +++ b/src/server/error/error.go @@ -16,11 +16,14 @@ package error import ( "errors" - "net/http" - + "fmt" + openapi "github.com/go-openapi/errors" "github.com/go-openapi/runtime" "github.com/go-openapi/runtime/middleware" + "github.com/goharbor/harbor/src/common/utils/log" ierror "github.com/goharbor/harbor/src/internal/error" + "net/http" + "strings" ) var ( @@ -28,25 +31,57 @@ var ( ierror.BadRequestCode: http.StatusBadRequest, ierror.UnAuthorizedCode: http.StatusUnauthorized, ierror.ForbiddenCode: http.StatusForbidden, + ierror.DENIED: http.StatusForbidden, ierror.NotFoundCode: http.StatusNotFound, ierror.ConflictCode: http.StatusConflict, ierror.PreconditionCode: http.StatusPreconditionFailed, ierror.ViolateForeignKeyConstraintCode: http.StatusPreconditionFailed, + ierror.PROJECTPOLICYVIOLATION: http.StatusPreconditionFailed, ierror.GeneralCode: http.StatusInternalServerError, } ) -// APIError generates the HTTP status code and error payload based on the input err -func APIError(err error) (int, string) { - return getHTTPStatusCode(ierror.ErrCode(err)), ierror.NewErrs(err).Error() +// TODO use "SendError" instead in the v1 APIs? + +// SendError tries to parse the HTTP status code from the specified error, envelops it into +// an error array as the error payload and returns the code and payload to the response. +// And the error is logged as well +func SendError(w http.ResponseWriter, err error) { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + statusCode, errPayload := apiError(err) + // the error detail is logged only, and will not be sent to the client to avoid leaking server information + if statusCode >= http.StatusInternalServerError { + log.Error(errPayload) + err = ierror.New(nil).WithCode(ierror.GeneralCode).WithMessage("internal server error") + errPayload = ierror.NewErrs(err).Error() + } else { + // only log the error whose status code < 500 when debugging to avoid log flooding + log.Debug(errPayload) + } + w.WriteHeader(statusCode) + fmt.Fprintln(w, errPayload) } -func getHTTPStatusCode(errCode string) int { - statusCode, ok := codeMap[errCode] - if !ok { - statusCode = http.StatusInternalServerError +// generates the HTTP status code based on the specified error, +// envelops the error into an error array as the payload and return them +func apiError(err error) (statusCode int, errPayload string) { + code := 0 + var openAPIErr openapi.Error + if errors.As(err, &openAPIErr) { + // Before executing operation handler, go-swagger will bind a parameters object to a request and validate the request, + // it will return directly when bind and validate failed. + // The response format of the default ServeError implementation does not match the internal error response format. + // So we needed to convert the format to the internal error response format. + code = int(openAPIErr.Code()) + errCode := strings.Replace(strings.ToUpper(http.StatusText(code)), " ", "_", -1) + err = ierror.New(nil).WithCode(errCode).WithMessage(openAPIErr.Error()) + } else { + code = codeMap[ierror.ErrCode(err)] } - return statusCode + if code == 0 { + code = http.StatusInternalServerError + } + return code, ierror.NewErrs(err).Error() } var _ middleware.Responder = &ErrResponder{} @@ -58,20 +93,7 @@ type ErrResponder struct { // WriteResponse ... func (r *ErrResponder) WriteResponse(rw http.ResponseWriter, producer runtime.Producer) { - code := ierror.ErrCode(r.err) - rw.WriteHeader(getHTTPStatusCode(code)) - - var e *ierror.Error - if !errors.As(r.err, &e) { - e = &ierror.Error{ - Code: code, - Message: r.err.Error(), - } - } - - if err := producer.Produce(rw, e); err != nil { - panic(err) // let the recovery middleware deal with this - } + SendError(rw, r.err) } // NewErrResponder returns responder for err diff --git a/src/server/error/error_test.go b/src/server/error/error_test.go index 7b46f09586..905c2a0d7a 100644 --- a/src/server/error/error_test.go +++ b/src/server/error/error_test.go @@ -16,38 +16,51 @@ package error import ( "errors" + openapi "github.com/go-openapi/errors" ierror "github.com/goharbor/harbor/src/internal/error" "github.com/stretchr/testify/assert" "net/http" + "net/http/httptest" "testing" ) -func TestGetHTTPStatusCode(t *testing.T) { - // pre-defined error code - errCode := ierror.NotFoundCode - statusCode := getHTTPStatusCode(errCode) - assert.Equal(t, http.StatusNotFound, statusCode) +func TestSendError(t *testing.T) { + // internal server error + rw := httptest.NewRecorder() + err := ierror.New(nil).WithCode(ierror.GeneralCode).WithMessage("unknown") + SendError(rw, err) + assert.Equal(t, http.StatusInternalServerError, rw.Code) + assert.Equal(t, `{"errors":[{"code":"UNKNOWN","message":"internal server error"}]}`+"\n", rw.Body.String()) - // not-defined error code - errCode = "NOT_DEFINED_ERROR_CODE" - statusCode = getHTTPStatusCode(errCode) - assert.Equal(t, http.StatusInternalServerError, statusCode) + // not internal server error + rw = httptest.NewRecorder() + err = ierror.New(nil).WithCode(ierror.NotFoundCode).WithMessage("object not found") + SendError(rw, err) + assert.Equal(t, http.StatusNotFound, rw.Code) + assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"object not found"}]}`+"\n", rw.Body.String()) } func TestAPIError(t *testing.T) { + var err error + // open API error: github.com/go-openapi/errors.Error + err = openapi.New(400, "bad request") + statusCode, payload := apiError(err) + assert.Equal(t, http.StatusBadRequest, statusCode) + assert.Equal(t, `{"errors":[{"code":"BAD_REQUEST","message":"bad request"}]}`, payload) + // ierror.Error - err := &ierror.Error{ + err = &ierror.Error{ Cause: nil, Code: ierror.NotFoundCode, Message: "resource not found", } - statusCode, payload := APIError(err) + statusCode, payload = apiError(err) assert.Equal(t, http.StatusNotFound, statusCode) assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"resource not found"}]}`, payload) // common error e := errors.New("customized error") - statusCode, payload = APIError(e) + statusCode, payload = apiError(e) assert.Equal(t, http.StatusInternalServerError, statusCode) assert.Equal(t, `{"errors":[{"code":"UNKNOWN","message":"customized error"}]}`, payload) } diff --git a/src/server/middleware/artifactinfo/artifact_info.go b/src/server/middleware/artifactinfo/artifact_info.go index 7e116512da..b6ced8f0d6 100644 --- a/src/server/middleware/artifactinfo/artifact_info.go +++ b/src/server/middleware/artifactinfo/artifact_info.go @@ -17,16 +17,15 @@ package artifactinfo import ( "context" "fmt" + "github.com/goharbor/harbor/src/common/utils/log" + ierror "github.com/goharbor/harbor/src/internal/error" + serror "github.com/goharbor/harbor/src/server/error" + "github.com/goharbor/harbor/src/server/middleware" + "github.com/opencontainers/go-digest" "net/http" "net/url" "regexp" "strings" - - "github.com/goharbor/harbor/src/common/utils/log" - ierror "github.com/goharbor/harbor/src/internal/error" - "github.com/goharbor/harbor/src/server/middleware" - reg_err "github.com/goharbor/harbor/src/server/registry/error" - "github.com/opencontainers/go-digest" ) const ( @@ -58,7 +57,7 @@ func Middleware() func(http.Handler) http.Handler { repo := m[middleware.RepositorySubexp] pn, err := projectNameFromRepo(repo) if err != nil { - reg_err.Handle(rw, req, ierror.BadRequestError(err)) + serror.SendError(rw, ierror.BadRequestError(err)) return } art := &middleware.ArtifactInfo{ @@ -77,7 +76,7 @@ func Middleware() func(http.Handler) http.Handler { // it's not clear in OCI spec how to handle invalid from parm bmp, err := projectNameFromRepo(bmr) if err != nil { - reg_err.Handle(rw, req, ierror.BadRequestError(err)) + serror.SendError(rw, ierror.BadRequestError(err)) return } art.BlobMountDigest = m[blobMountDigest] diff --git a/src/server/middleware/contenttrust/contenttrust.go b/src/server/middleware/contenttrust/contenttrust.go index 4183c4ac58..71894a0529 100644 --- a/src/server/middleware/contenttrust/contenttrust.go +++ b/src/server/middleware/contenttrust/contenttrust.go @@ -6,6 +6,7 @@ import ( "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/middlewares/util" internal_errors "github.com/goharbor/harbor/src/internal/error" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" "net/http" "net/http/httptest" @@ -28,16 +29,12 @@ func Middleware() func(http.Handler) http.Handler { if rec.Result().StatusCode == http.StatusOK { match, err := matchNotaryDigest(mf) if err != nil { - pkgE := internal_errors.New(nil).WithCode(internal_errors.PROJECTPOLICYVIOLATION).WithMessage("Failed in communication with Notary please check the log") - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusInternalServerError) + serror.SendError(rw, err) return } if !match { - log.Debugf("digest mismatch, failing the response.") pkgE := internal_errors.New(nil).WithCode(internal_errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Notary.") - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusPreconditionFailed) + serror.SendError(rw, pkgE) return } } diff --git a/src/server/middleware/immutable/deletemf.go b/src/server/middleware/immutable/deletemf.go index dc908d0fdb..4373b1a335 100644 --- a/src/server/middleware/immutable/deletemf.go +++ b/src/server/middleware/immutable/deletemf.go @@ -6,6 +6,7 @@ import ( "github.com/goharbor/harbor/src/api/artifact" common_util "github.com/goharbor/harbor/src/common/utils" internal_errors "github.com/goharbor/harbor/src/internal/error" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" "net/http" ) @@ -18,13 +19,12 @@ func MiddlewareDelete() func(http.Handler) http.Handler { var e *ErrImmutable if errors.As(err, &e) { pkgE := internal_errors.New(e).WithCode(internal_errors.PreconditionCode) - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusPreconditionFailed) + serror.SendError(rw, pkgE) return } pkgE := internal_errors.New(fmt.Errorf("error occurred when to handle request in immutable handler: %v", err)).WithCode(internal_errors.GeneralCode) - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusInternalServerError) + serror.SendError(rw, pkgE) + return } next.ServeHTTP(rw, req) }) diff --git a/src/server/middleware/immutable/pushmf.go b/src/server/middleware/immutable/pushmf.go index 6964fa4aeb..fdbeae630f 100644 --- a/src/server/middleware/immutable/pushmf.go +++ b/src/server/middleware/immutable/pushmf.go @@ -6,6 +6,7 @@ import ( "github.com/goharbor/harbor/src/api/artifact" common_util "github.com/goharbor/harbor/src/common/utils" internal_errors "github.com/goharbor/harbor/src/internal/error" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" "net/http" ) @@ -18,13 +19,12 @@ func MiddlewarePush() func(http.Handler) http.Handler { var e *ErrImmutable if errors.As(err, &e) { pkgE := internal_errors.New(e).WithCode(internal_errors.PreconditionCode) - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusPreconditionFailed) + serror.SendError(rw, pkgE) return } pkgE := internal_errors.New(fmt.Errorf("error occurred when to handle request in immutable handler: %v", err)).WithCode(internal_errors.GeneralCode) - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusInternalServerError) + serror.SendError(rw, pkgE) + return } next.ServeHTTP(rw, req) }) diff --git a/src/server/middleware/manifestinfo/manifest_info.go b/src/server/middleware/manifestinfo/manifest_info.go index c522cde38e..3e3308adb2 100644 --- a/src/server/middleware/manifestinfo/manifest_info.go +++ b/src/server/middleware/manifestinfo/manifest_info.go @@ -5,8 +5,8 @@ import ( "github.com/goharbor/harbor/src/common/utils" ierror "github.com/goharbor/harbor/src/internal/error" project2 "github.com/goharbor/harbor/src/pkg/project" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" - reg_err "github.com/goharbor/harbor/src/server/registry/error" "github.com/opencontainers/go-digest" "net/http" "regexp" @@ -23,7 +23,7 @@ func Middleware() func(http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { mf, err := parseManifestInfoFromPath(req) if err != nil { - reg_err.Handle(rw, req, err) + serror.SendError(rw, err) return } *req = *(req.WithContext(middleware.NewManifestInfoContext(req.Context(), mf))) diff --git a/src/server/middleware/readonly/readonly.go b/src/server/middleware/readonly/readonly.go index 07b65c1f2e..490c0fad57 100644 --- a/src/server/middleware/readonly/readonly.go +++ b/src/server/middleware/readonly/readonly.go @@ -15,9 +15,9 @@ package readonly import ( + serror "github.com/goharbor/harbor/src/server/error" "net/http" - "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/config" internal_errors "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/server/middleware" @@ -67,9 +67,8 @@ func MiddlewareWithConfig(config Config, skippers ...middleware.Skipper) func(ht return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) { if config.ReadOnly(r) { - log.Warningf("The request is prohibited in readonly mode, url is: %s", r.URL.Path) pkgE := internal_errors.New(nil).WithCode(internal_errors.DENIED).WithMessage("The system is in read only mode. Any modification is prohibited.") - http.Error(w, internal_errors.NewErrs(pkgE).Error(), http.StatusForbidden) + serror.SendError(w, pkgE) return } diff --git a/src/server/middleware/regtoken/regtoken.go b/src/server/middleware/regtoken/regtoken.go index f4388a9b78..35160b2817 100644 --- a/src/server/middleware/regtoken/regtoken.go +++ b/src/server/middleware/regtoken/regtoken.go @@ -7,8 +7,8 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" pkg_token "github.com/goharbor/harbor/src/pkg/token" "github.com/goharbor/harbor/src/pkg/token/claims/registry" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" - reg_err "github.com/goharbor/harbor/src/server/registry/error" "net/http" "strings" ) @@ -19,7 +19,7 @@ func Middleware() func(http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { err := parseToken(req) if err != nil { - reg_err.Handle(rw, req, err) + serror.SendError(rw, err) return } next.ServeHTTP(rw, req) diff --git a/src/server/middleware/transaction/transaction.go b/src/server/middleware/transaction/transaction.go index f7dd8f25d9..f42f66b52f 100644 --- a/src/server/middleware/transaction/transaction.go +++ b/src/server/middleware/transaction/transaction.go @@ -18,6 +18,7 @@ import ( "context" "errors" "fmt" + serror "github.com/goharbor/harbor/src/server/error" "net/http" "github.com/goharbor/harbor/src/common/utils/log" @@ -77,7 +78,7 @@ func Middleware(skippers ...middleware.Skipper) func(http.Handler) http.Handler } if err := orm.WithTransaction(h)(r.Context()); err != nil && err != errNonSuccess { - log.Errorf("deal with %s request in transaction failed: %v", r.URL.Path, err) + err = fmt.Errorf("deal with %s request in transaction failed: %v", r.URL.Path, err) // begin, commit or rollback transaction db error happened, // reset the response and set status code to 500 @@ -85,7 +86,7 @@ func Middleware(skippers ...middleware.Skipper) func(http.Handler) http.Handler log.Errorf("reset the response failed: %v", err) return } - res.WriteHeader(http.StatusInternalServerError) + serror.SendError(res, err) } }, skippers...) } diff --git a/src/server/middleware/v2auth/auth.go b/src/server/middleware/v2auth/auth.go index ad40b3502d..6eb956d325 100644 --- a/src/server/middleware/v2auth/auth.go +++ b/src/server/middleware/v2auth/auth.go @@ -17,6 +17,7 @@ package v2auth import ( "context" "fmt" + serror "github.com/goharbor/harbor/src/server/error" "net/http" "sync" @@ -27,7 +28,6 @@ import ( "github.com/goharbor/harbor/src/core/promgr" ierror "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/server/middleware" - reg_err "github.com/goharbor/harbor/src/server/registry/error" ) type reqChecker struct { @@ -131,7 +131,7 @@ func Middleware() func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { if err := checker.check(req); err != nil { - reg_err.Handle(rw, req, ierror.UnauthorizedError(err).WithMessage(err.Error())) + serror.SendError(rw, ierror.UnauthorizedError(err).WithMessage(err.Error())) return } next.ServeHTTP(rw, req) diff --git a/src/server/middleware/vulnerable/vulnerable.go b/src/server/middleware/vulnerable/vulnerable.go index c92d866d5b..448df3492e 100644 --- a/src/server/middleware/vulnerable/vulnerable.go +++ b/src/server/middleware/vulnerable/vulnerable.go @@ -8,6 +8,7 @@ import ( "github.com/goharbor/harbor/src/pkg/scan/report" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" "github.com/goharbor/harbor/src/pkg/scan/vuln" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/middleware" "github.com/pkg/errors" "net/http" @@ -30,7 +31,8 @@ func Middleware() func(http.Handler) http.Handler { // Invalid project ID if wl.ProjectID == 0 { err := errors.Errorf("project verification error: project %s", img.ProjectName) - sendError(err, rw) + pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION) + serror.SendError(rw, pkgE) return } @@ -52,7 +54,8 @@ func Middleware() func(http.Handler) http.Handler { if err != nil { err = errors.Wrap(err, "middleware: vulnerable handler") - sendError(err, rw) + pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION) + serror.SendError(rw, pkgE) return } @@ -60,7 +63,8 @@ func Middleware() func(http.Handler) http.Handler { // No report yet? if !ok { err = errors.Errorf("no scan report existing for the artifact: %s:%s@%s", img.Repository, img.Tag, img.Digest) - sendError(err, rw) + pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION) + serror.SendError(rw, pkgE) return } @@ -70,7 +74,8 @@ func Middleware() func(http.Handler) http.Handler { if summary.Severity.Code() >= projectVulnerableSeverity.Code() { err = errors.Errorf("current image with '%q vulnerable' cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of %q from running.' "+ "Please contact your project administrator for help'", summary.Severity, projectVulnerableSeverity) - sendError(err, rw) + pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION) + serror.SendError(rw, pkgE) return } @@ -110,10 +115,3 @@ func validate(req *http.Request) (bool, *middleware.ManifestInfo, vuln.Severity, } return true, mf, projectVulnerableSeverity, wl } - -func sendError(err error, rw http.ResponseWriter) { - log.Error(err) - pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION) - msg := internal_errors.NewErrs(pkgE).Error() - http.Error(rw, msg, http.StatusPreconditionFailed) -} diff --git a/src/server/registry/catalog.go b/src/server/registry/catalog.go index e27af42695..22c341d546 100644 --- a/src/server/registry/catalog.go +++ b/src/server/registry/catalog.go @@ -19,7 +19,7 @@ import ( "fmt" "github.com/goharbor/harbor/src/api/repository" ierror "github.com/goharbor/harbor/src/internal/error" - reg_error "github.com/goharbor/harbor/src/server/registry/error" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/registry/util" "net/http" "sort" @@ -47,7 +47,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) maxEntries, err = strconv.Atoi(reqQ.Get("n")) if err != nil || maxEntries < 0 { err := ierror.New(err).WithCode(ierror.BadRequestCode).WithMessage("the N must be a positive int type") - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } } @@ -57,7 +57,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) // ToDo filter out the untagged repos total, repoRecords, err := r.repoCtl.List(req.Context(), nil) if err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } if total <= 0 { @@ -81,7 +81,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) lastEntryIndex := util.IndexString(repoNames, lastEntry) if lastEntryIndex == -1 { err := ierror.New(nil).WithCode(ierror.BadRequestCode).WithMessage(fmt.Sprintf("the last: %s should be a valid repository name.", lastEntry)) - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } resRepos = repoNames[lastEntryIndex+1 : lastEntryIndex+maxEntries] @@ -94,7 +94,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) if repoNames[len(repoNames)-1] != resRepos[len(resRepos)-1] { urlStr, err := util.SetLinkHeader(req.URL.String(), maxEntries, resRepos[len(resRepos)-1]) if err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } w.Header().Set("Link", urlStr) @@ -111,7 +111,7 @@ func (r *repositoryHandler) sendResponse(w http.ResponseWriter, req *http.Reques if err := enc.Encode(catalogAPIResponse{ Repositories: repositoryNames, }); err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } } diff --git a/src/server/registry/error/error.go b/src/server/registry/error/error.go deleted file mode 100644 index 969b1390c8..0000000000 --- a/src/server/registry/error/error.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package error - -import ( - "github.com/goharbor/harbor/src/common/utils/log" - serror "github.com/goharbor/harbor/src/server/error" - "net/http" -) - -// Handle generates the HTTP status code and error payload and writes them to the response -func Handle(w http.ResponseWriter, req *http.Request, err error) { - log.Errorf("failed to handle the request %s %s: %v", req.Method, req.URL, err) - statusCode, payload := serror.APIError(err) - w.WriteHeader(statusCode) - w.Write([]byte(payload)) -} diff --git a/src/server/registry/manifest.go b/src/server/registry/manifest.go index cdc63d6aac..217824d480 100644 --- a/src/server/registry/manifest.go +++ b/src/server/registry/manifest.go @@ -19,7 +19,7 @@ import ( "github.com/goharbor/harbor/src/api/repository" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/internal" - "github.com/goharbor/harbor/src/server/registry/error" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/router" "github.com/opencontainers/go-digest" "net/http" @@ -32,7 +32,7 @@ func getManifest(w http.ResponseWriter, req *http.Request) { reference := router.Param(req.Context(), ":reference") artifact, err := artifact.Ctl.GetByReference(req.Context(), repository, reference, nil) if err != nil { - error.Handle(w, req, err) + serror.SendError(w, err) return } @@ -53,11 +53,11 @@ func deleteManifest(w http.ResponseWriter, req *http.Request) { reference := router.Param(req.Context(), ":reference") art, err := artifact.Ctl.GetByReference(req.Context(), repository, reference, nil) if err != nil { - error.Handle(w, req, err) + serror.SendError(w, err) return } if err = artifact.Ctl.Delete(req.Context(), art.ID); err != nil { - error.Handle(w, req, err) + serror.SendError(w, err) return } w.WriteHeader(http.StatusAccepted) @@ -72,7 +72,7 @@ func putManifest(w http.ResponseWriter, req *http.Request) { // make sure the repository exist before pushing the manifest _, repositoryID, err := repository.Ctl.Ensure(req.Context(), repo) if err != nil { - error.Handle(w, req, err) + serror.SendError(w, err) return } @@ -100,7 +100,7 @@ func putManifest(w http.ResponseWriter, req *http.Request) { _, _, err = artifact.Ctl.Ensure(req.Context(), repositoryID, dgt, tags...) if err != nil { - error.Handle(w, req, err) + serror.SendError(w, err) return } diff --git a/src/server/registry/tag.go b/src/server/registry/tag.go index 82d88f6393..a87a1675f6 100644 --- a/src/server/registry/tag.go +++ b/src/server/registry/tag.go @@ -21,7 +21,7 @@ import ( "github.com/goharbor/harbor/src/api/repository" ierror "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/q" - reg_error "github.com/goharbor/harbor/src/server/registry/error" + serror "github.com/goharbor/harbor/src/server/error" "github.com/goharbor/harbor/src/server/registry/util" "github.com/goharbor/harbor/src/server/router" "net/http" @@ -65,7 +65,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { maxEntries, err = strconv.Atoi(reqQ.Get("n")) if err != nil || maxEntries < 0 { err := ierror.New(err).WithCode(ierror.BadRequestCode).WithMessage("the N must be a positive int type") - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } } @@ -75,7 +75,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { t.repositoryName = router.Param(req.Context(), ":splat") repository, err := t.repoCtl.GetByName(req.Context(), t.repositoryName) if err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } @@ -85,7 +85,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { "RepositoryID": repository.RepositoryID, }}, nil) if err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } if total == 0 { @@ -110,7 +110,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { lastEntryIndex := util.IndexString(tagNames, lastEntry) if lastEntryIndex == -1 { err := ierror.New(nil).WithCode(ierror.BadRequestCode).WithMessage(fmt.Sprintf("the last: %s should be a valid tag name.", lastEntry)) - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } resTags = tagNames[lastEntryIndex+1 : lastEntryIndex+maxEntries] @@ -123,7 +123,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { if tagNames[len(tagNames)-1] != resTags[len(resTags)-1] { urlStr, err := util.SetLinkHeader(req.URL.String(), maxEntries, resTags[len(resTags)-1]) if err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } w.Header().Set("Link", urlStr) @@ -140,7 +140,7 @@ func (t *tagHandler) sendResponse(w http.ResponseWriter, req *http.Request, tagN Name: t.repositoryName, Tags: tagNames, }); err != nil { - reg_error.Handle(w, req, err) + serror.SendError(w, err) return } } diff --git a/src/server/v2.0/handler/handler.go b/src/server/v2.0/handler/handler.go index 157f3b0437..dde53d47fb 100644 --- a/src/server/v2.0/handler/handler.go +++ b/src/server/v2.0/handler/handler.go @@ -15,16 +15,10 @@ package handler import ( - "encoding/json" - "fmt" + serror "github.com/goharbor/harbor/src/server/error" + "github.com/goharbor/harbor/src/server/v2.0/restapi" "log" "net/http" - "net/http/httptest" - "strings" - - "github.com/go-openapi/errors" - ierrors "github.com/goharbor/harbor/src/internal/error" - "github.com/goharbor/harbor/src/server/v2.0/restapi" ) // New returns http handler for API V2.0 @@ -42,31 +36,10 @@ func New() http.Handler { return h } -type apiError struct { - Code int32 `json:"code"` - Message string `json:"message"` -} - // Before executing operation handler, go-swagger will bind a parameters object to a request and validate the request, // it will return directly when bind and validate failed. // The response format of the default ServeError implementation does not match the internal error response format. // So we needed to convert the format to the internal error response format. func serveError(rw http.ResponseWriter, r *http.Request, err error) { - w := httptest.NewRecorder() - errors.ServeError(w, r, err) - - rw.WriteHeader(w.Code) - for key, values := range w.Header() { - for _, value := range values { - rw.Header().Add(key, value) - } - } - - var er apiError - json.Unmarshal(w.Body.Bytes(), &er) - - code := strings.Replace(strings.ToUpper(http.StatusText(w.Code)), " ", "_", -1) - - e := ierrors.New(fmt.Errorf(er.Message)).WithCode(code) - rw.Write([]byte(e.Error())) + serror.SendError(rw, err) }