diff --git a/src/common/api/base.go b/src/common/api/base.go index 9049538a2..f8f12db0b 100644 --- a/src/common/api/base.go +++ b/src/common/api/base.go @@ -20,11 +20,12 @@ import ( "net/http" "strconv" + "errors" "github.com/astaxie/beego" "github.com/astaxie/beego/validation" commonhttp "github.com/goharbor/harbor/src/common/http" "github.com/goharbor/harbor/src/common/utils/log" - "github.com/pkg/errors" + internal_errors "github.com/goharbor/harbor/src/internal/error" ) const ( @@ -248,3 +249,48 @@ func (b *BaseAPI) SendPreconditionFailedError(err error) { func (b *BaseAPI) SendStatusServiceUnavailableError(err error) { b.RenderFormattedError(http.StatusServiceUnavailable, err.Error()) } + +// SendError return the error defined in OCI spec: https://github.com/opencontainers/distribution-spec/blob/master/spec.md#errors +// { +// "errors:" [{ +// "code": , +// "message": , +// // optional +// "detail": +// }, +// ... +// ] +// } +func (b *BaseAPI) SendError(err error) { + var statusCode int + var send error + + var e *internal_errors.Error + if errors.As(err, &e) { + code := e.Code + statusCode = b.getStatusCode(code) + if statusCode == 0 { + statusCode = http.StatusInternalServerError + } + send = e + + } else { + statusCode = http.StatusInternalServerError + send = internal_errors.UnknownError(err) + } + + b.RenderError(statusCode, internal_errors.NewErrs(send).Error()) +} + +func (b *BaseAPI) getStatusCode(code string) int { + statusCodeMap := map[string]int{ + internal_errors.NotFoundCode: http.StatusNotFound, + internal_errors.ConflictCode: http.StatusConflict, + internal_errors.UnAuthorizedCode: http.StatusUnauthorized, + internal_errors.BadRequestCode: http.StatusBadRequest, + internal_errors.ForbiddenCode: http.StatusForbidden, + internal_errors.PreconditionCode: http.StatusPreconditionFailed, + internal_errors.GeneralCode: http.StatusInternalServerError, + } + return statusCodeMap[code] +} diff --git a/src/common/dao/base.go b/src/common/dao/base.go index 43ded29ef..9d74eebc2 100644 --- a/src/common/dao/base.go +++ b/src/common/dao/base.go @@ -150,9 +150,9 @@ func GetOrmer() orm.Ormer { return globalOrm } -// isDupRecErr checks if the error is due to a duplication of record, currently this +// IsDupRecErr checks if the error is due to a duplication of record, currently this // works only for pgSQL -func isDupRecErr(e error) bool { +func IsDupRecErr(e error) bool { return strings.Contains(e.Error(), "duplicate key value violates unique constraint") } diff --git a/src/common/dao/config.go b/src/common/dao/config.go index eea49cb30..c12111e9c 100644 --- a/src/common/dao/config.go +++ b/src/common/dao/config.go @@ -61,7 +61,7 @@ func SaveConfigEntries(entries []models.ConfigEntry) error { tempEntry.Key = entry.Key tempEntry.Value = entry.Value created, _, err := o.ReadOrCreate(&tempEntry, "k") - if err != nil && !isDupRecErr(err) { + if err != nil && !IsDupRecErr(err) { log.Errorf("Error create configuration entry: %v", err) return err } diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go index b0f216059..788b4f1d4 100644 --- a/src/common/dao/dao_test.go +++ b/src/common/dao/dao_test.go @@ -864,8 +864,8 @@ func TestSaveConfigEntries(t *testing.T) { } func TestIsDupRecError(t *testing.T) { - assert.True(t, isDupRecErr(fmt.Errorf("pq: duplicate key value violates unique constraint \"properties_k_key\""))) - assert.False(t, isDupRecErr(fmt.Errorf("other error"))) + assert.True(t, IsDupRecErr(fmt.Errorf("pq: duplicate key value violates unique constraint \"properties_k_key\""))) + assert.False(t, IsDupRecErr(fmt.Errorf("other error"))) } func TestWithTransaction(t *testing.T) { diff --git a/src/core/api/base.go b/src/core/api/base.go index 5139edfc2..bf94c54f0 100644 --- a/src/core/api/base.go +++ b/src/core/api/base.go @@ -30,6 +30,7 @@ import ( "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/filter" "github.com/goharbor/harbor/src/core/promgr" + internal_errors "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/project" "github.com/goharbor/harbor/src/pkg/repository" "github.com/goharbor/harbor/src/pkg/retention" @@ -93,7 +94,7 @@ func (b *BaseController) Prepare() { // otherwise send Unauthorized response and returns false func (b *BaseController) RequireAuthenticated() bool { if !b.SecurityCtx.IsAuthenticated() { - b.SendUnAuthorizedError(errors.New("Unauthorized")) + b.SendError(internal_errors.UnauthorizedError(errors.New("Unauthorized"))) return false } @@ -132,10 +133,10 @@ func (b *BaseController) HasProjectPermission(projectIDOrName interface{}, actio func (b *BaseController) RequireProjectAccess(projectIDOrName interface{}, action rbac.Action, subresource ...rbac.Resource) bool { hasPermission, err := b.HasProjectPermission(projectIDOrName, action, subresource...) if err != nil { - if err == errNotFound { - b.SendNotFoundError(fmt.Errorf("project %v not found", projectIDOrName)) + if errors.Is(err, errNotFound) { + b.SendError(internal_errors.New(errors.New(b.SecurityCtx.GetUsername())).WithCode(internal_errors.NotFoundCode)) } else { - b.SendInternalServerError(err) + b.SendError(err) } return false @@ -143,9 +144,9 @@ func (b *BaseController) RequireProjectAccess(projectIDOrName interface{}, actio if !hasPermission { if !b.SecurityCtx.IsAuthenticated() { - b.SendUnAuthorizedError(errors.New("UnAuthorized")) + b.SendError(internal_errors.UnauthorizedError(errors.New("Unauthorized"))) } else { - b.SendForbiddenError(errors.New(b.SecurityCtx.GetUsername())) + b.SendError(internal_errors.New(errors.New(b.SecurityCtx.GetUsername())).WithCode(internal_errors.ForbiddenCode)) } return false diff --git a/src/core/api/immutabletagrule.go b/src/core/api/immutabletagrule.go index fdd296457..8b1435eff 100644 --- a/src/core/api/immutabletagrule.go +++ b/src/core/api/immutabletagrule.go @@ -1,13 +1,13 @@ package api import ( - "errors" "fmt" "net/http" "strconv" "strings" "github.com/goharbor/harbor/src/common/rbac" + internal_errors "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/immutabletag" "github.com/goharbor/harbor/src/pkg/immutabletag/model" ) @@ -23,8 +23,8 @@ type ImmutableTagRuleAPI struct { // Prepare validates the user and projectID func (itr *ImmutableTagRuleAPI) Prepare() { itr.BaseController.Prepare() - if !itr.SecurityCtx.IsAuthenticated() { - itr.SendUnAuthorizedError(errors.New("Unauthorized")) + // Check access permissions + if !itr.RequireAuthenticated() { return } @@ -36,7 +36,7 @@ func (itr *ImmutableTagRuleAPI) Prepare() { } else { text += fmt.Sprintf("%d", pid) } - itr.SendBadRequestError(errors.New(text)) + itr.SendError(internal_errors.New(err).WithCode(internal_errors.BadRequestCode)) return } itr.projectID = pid @@ -46,15 +46,14 @@ func (itr *ImmutableTagRuleAPI) Prepare() { itr.ID = ruleID itRule, err := itr.ctr.GetImmutableRule(itr.ID) if err != nil { - itr.SendInternalServerError(err) + itr.SendError(err) return } - if itRule == nil || itRule.ProjectID != itr.projectID { + if itRule.ProjectID != itr.projectID { err := fmt.Errorf("immutable tag rule %v not found", itr.ID) - itr.SendNotFoundError(err) + itr.SendError(internal_errors.New(err).WithCode(internal_errors.NotFoundCode)) return } - } if strings.EqualFold(itr.Ctx.Request.Method, "get") { @@ -85,7 +84,7 @@ func (itr *ImmutableTagRuleAPI) requireAccess(action rbac.Action) bool { func (itr *ImmutableTagRuleAPI) List() { rules, err := itr.ctr.ListImmutableRules(itr.projectID) if err != nil { - itr.SendInternalServerError(err) + itr.SendError(err) return } itr.WriteJSONData(rules) @@ -96,32 +95,28 @@ func (itr *ImmutableTagRuleAPI) Post() { ir := &model.Metadata{} isValid, err := itr.DecodeJSONReqAndValidate(ir) if !isValid { - itr.SendBadRequestError(err) + itr.SendError(internal_errors.New(err).WithCode(internal_errors.BadRequestCode)) return } ir.ProjectID = itr.projectID id, err := itr.ctr.CreateImmutableRule(ir) - if err != nil && strings.Contains(err.Error(), "duplicate key") { - itr.RenderError(http.StatusConflict, "immutable tag rule duplicated") - return - } if err != nil { - itr.SendInternalServerError(err) + itr.SendError(err) return } itr.Redirect(http.StatusCreated, strconv.FormatInt(id, 10)) - } // Delete delete immutable tag rule func (itr *ImmutableTagRuleAPI) Delete() { if itr.ID <= 0 { - itr.SendBadRequestError(fmt.Errorf("invalid immutable rule id %d", itr.ID)) + err := fmt.Errorf("invalid immutable rule id %d", itr.ID) + itr.SendError(internal_errors.New(err).WithCode(internal_errors.BadRequestCode)) return } err := itr.ctr.DeleteImmutableRule(itr.ID) if err != nil { - itr.SendInternalServerError(err) + itr.SendError(err) return } } @@ -130,19 +125,20 @@ func (itr *ImmutableTagRuleAPI) Delete() { func (itr *ImmutableTagRuleAPI) Put() { ir := &model.Metadata{} if err := itr.DecodeJSONReq(ir); err != nil { - itr.SendBadRequestError(err) + itr.SendError(internal_errors.New(err).WithCode(internal_errors.BadRequestCode)) return } ir.ID = itr.ID ir.ProjectID = itr.projectID if itr.ID <= 0 { - itr.SendBadRequestError(fmt.Errorf("invalid immutable rule id %d", itr.ID)) + err := fmt.Errorf("invalid immutable rule id %d", itr.ID) + itr.SendError(internal_errors.New(err).WithCode(internal_errors.BadRequestCode)) return } if err := itr.ctr.UpdateImmutableRule(itr.projectID, ir); err != nil { - itr.SendInternalServerError(err) + itr.SendError(err) return } } diff --git a/src/core/middlewares/immutable/handler.go b/src/core/middlewares/immutable/handler.go index e60eb8ab2..f044ecad0 100644 --- a/src/core/middlewares/immutable/handler.go +++ b/src/core/middlewares/immutable/handler.go @@ -15,11 +15,12 @@ package immutable import ( + "errors" "fmt" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/middlewares/interceptor" - "github.com/goharbor/harbor/src/core/middlewares/util" middlerware_err "github.com/goharbor/harbor/src/core/middlewares/util/error" + internal_errors "github.com/goharbor/harbor/src/internal/error" "net/http" ) @@ -46,8 +47,9 @@ func (rh *immutableHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) interceptor, err := rh.getInterceptor(req) if err != nil { log.Warningf("Error occurred when to handle request in immutable handler: %v", err) - http.Error(rw, util.MarshalError("InternalError", fmt.Sprintf("Error occurred when to handle request in immutable handler: %v", err)), - http.StatusInternalServerError) + 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) return } @@ -58,13 +60,17 @@ func (rh *immutableHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) if err := interceptor.HandleRequest(req); err != nil { log.Warningf("Error occurred when to handle request in immutable handler: %v", err) - if _, ok := err.(middlerware_err.ErrImmutable); ok { - http.Error(rw, util.MarshalError("DENIED", - fmt.Sprintf("%v", err)), http.StatusPreconditionFailed) + var e *middlerware_err.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) return } - http.Error(rw, util.MarshalError("InternalError", fmt.Sprintf("Error occurred when to handle request in immutable handler: %v", err)), - http.StatusInternalServerError) + + 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) return } diff --git a/src/core/middlewares/util/error/immutable.go b/src/core/middlewares/util/error/immutable.go index 82b3799d2..8230b7167 100644 --- a/src/core/middlewares/util/error/immutable.go +++ b/src/core/middlewares/util/error/immutable.go @@ -11,13 +11,18 @@ type ErrImmutable struct { } // Error ... -func (ei ErrImmutable) Error() string { +func (ei *ErrImmutable) Error() string { return fmt.Sprintf("Failed to process request due to '%s:%s' configured as immutable.", ei.repo, ei.tag) } +// Unwrap ... +func (ei *ErrImmutable) Unwrap() error { + return nil +} + // NewErrImmutable ... -func NewErrImmutable(msg, tag string) ErrImmutable { - return ErrImmutable{ +func NewErrImmutable(msg, tag string) error { + return &ErrImmutable{ repo: msg, tag: tag, } diff --git a/src/internal/error/errors.go b/src/internal/error/errors.go new file mode 100644 index 000000000..dc9df5087 --- /dev/null +++ b/src/internal/error/errors.go @@ -0,0 +1,147 @@ +package error + +import ( + "encoding/json" + "fmt" + "github.com/goharbor/harbor/src/common/utils/log" + "strings" +) + +// Error ... +type Error struct { + Cause error `json:"-"` + Code string `json:"code"` + Message string `json:"message"` +} + +// Error returns a human readable error. +func (e *Error) Error() string { + return fmt.Sprintf("%v, %s, %s", e.Cause, e.Code, e.Message) +} + +// WithMessage ... +func (e *Error) WithMessage(msg string) *Error { + e.Message = msg + return e +} + +// WithCode ... +func (e *Error) WithCode(code string) *Error { + e.Code = code + return e +} + +// Unwrap ... +func (e *Error) Unwrap() error { return e.Cause } + +// Errors ... +type Errors []error + +var _ error = Errors{} + +// Error converts slice of error +func (errs Errors) Error() string { + var tmpErrs struct { + Errors []Error `json:"errors,omitempty"` + } + + for _, e := range errs { + var err error + switch e.(type) { + case *Error: + err = e.(*Error) + default: + err = UnknownError(e).WithMessage(err.Error()) + } + tmpErrs.Errors = append(tmpErrs.Errors, *err.(*Error)) + } + + msg, err := json.Marshal(tmpErrs) + if err != nil { + log.Error(err) + return "{}" + } + return string(msg) +} + +// Len returns the current number of errors. +func (errs Errors) Len() int { + return len(errs) +} + +// NewErrs ... +func NewErrs(err error) Errors { + return Errors{err} +} + +const ( + // NotFoundCode is code for the error of no object found + NotFoundCode = "NOT_FOUND" + // ConflictCode ... + ConflictCode = "CONFLICT" + // UnAuthorizedCode ... + UnAuthorizedCode = "UNAUTHORIZED" + // BadRequestCode ... + BadRequestCode = "BAD_REQUEST" + // ForbiddenCode ... + ForbiddenCode = "FORBIDDER" + // PreconditionCode ... + PreconditionCode = "PRECONDITION" + // GeneralCode ... + GeneralCode = "UNKNOWN" +) + +// New ... +func New(err error) *Error { + if _, ok := err.(*Error); ok { + err = err.(*Error).Unwrap() + } + return &Error{ + Cause: err, + Message: err.Error(), + } +} + +// NotFoundError is error for the case of object not found +func NotFoundError(err error) *Error { + return New(err).WithCode(NotFoundCode).WithMessage("resource not found") +} + +// ConflictError is error for the case of object conflict +func ConflictError(err error) *Error { + return New(err).WithCode(ConflictCode).WithMessage("resource conflict") +} + +// UnauthorizedError is error for the case of unauthorized accessing +func UnauthorizedError(err error) *Error { + return New(err).WithCode(UnAuthorizedCode).WithMessage("unauthorized") +} + +// BadRequestError is error for the case of bad request +func BadRequestError(err error) *Error { + return New(err).WithCode(BadRequestCode).WithMessage("bad request") +} + +// ForbiddenError is error for the case of forbidden +func ForbiddenError(err error) *Error { + return New(err).WithCode(ForbiddenCode).WithMessage("forbidden") +} + +// PreconditionFailedError is error for the case of precondition failed +func PreconditionFailedError(err error) *Error { + return New(err).WithCode(PreconditionCode).WithMessage("preconfition") +} + +// UnknownError ... +func UnknownError(err error) *Error { + return New(err).WithCode(GeneralCode).WithMessage("unknown") +} + +// IsErr ... +func IsErr(err error, code string) bool { + _, ok := err.(*Error) + if !ok { + return false + } + return strings.Compare(err.(*Error).Code, code) == 0 +} diff --git a/src/pkg/immutabletag/dao/immutable.go b/src/pkg/immutabletag/dao/immutable.go index e7e338348..9e591767b 100644 --- a/src/pkg/immutabletag/dao/immutable.go +++ b/src/pkg/immutabletag/dao/immutable.go @@ -3,8 +3,10 @@ package dao import ( "fmt" + "errors" "github.com/astaxie/beego/orm" "github.com/goharbor/harbor/src/common/dao" + internal_errors "github.com/goharbor/harbor/src/internal/error" "github.com/goharbor/harbor/src/pkg/immutabletag/dao/model" ) @@ -30,21 +32,42 @@ type immutableRuleDao struct{} func (i *immutableRuleDao) CreateImmutableRule(ir *model.ImmutableRule) (int64, error) { ir.Disabled = false o := dao.GetOrmer() - return o.Insert(ir) + id, err := o.Insert(ir) + if err != nil { + if dao.IsDupRecErr(err) { + return id, internal_errors.ConflictError(err) + } + return id, err + } + return id, nil } // UpdateImmutableRule update the immutable rules func (i *immutableRuleDao) UpdateImmutableRule(projectID int64, ir *model.ImmutableRule) (int64, error) { ir.ProjectID = projectID o := dao.GetOrmer() - return o.Update(ir, "TagFilter") + id, err := o.Update(ir, "TagFilter") + if err != nil { + if errors.Is(err, orm.ErrNoRows) { + return id, internal_errors.NotFoundError(err) + } + return id, err + } + return id, nil } // ToggleImmutableRule enable/disable immutable rules func (i *immutableRuleDao) ToggleImmutableRule(id int64, status bool) (int64, error) { o := dao.GetOrmer() ir := &model.ImmutableRule{ID: id, Disabled: status} - return o.Update(ir, "Disabled") + id, err := o.Update(ir, "Disabled") + if err != nil { + if errors.Is(err, orm.ErrNoRows) { + return id, internal_errors.NotFoundError(err) + } + return id, err + } + return id, nil } // GetImmutableRule get immutable rule @@ -52,10 +75,11 @@ func (i *immutableRuleDao) GetImmutableRule(id int64) (*model.ImmutableRule, err o := dao.GetOrmer() ir := &model.ImmutableRule{ID: id} err := o.Read(ir) - if err == orm.ErrNoRows { - return nil, nil - } if err != nil { + if errors.Is(err, orm.ErrNoRows) { + return nil, internal_errors.New(err).WithCode(internal_errors.NotFoundCode). + WithMessage(fmt.Sprintf("the immutable rule %d is not found.", id)) + } return nil, err } return ir, nil @@ -68,7 +92,7 @@ func (i *immutableRuleDao) QueryImmutableRuleByProjectID(projectID int64) ([]mod r := make([]model.ImmutableRule, 0) _, err := qs.All(&r) if err != nil { - return nil, fmt.Errorf("failed to get immutable tag rule by projectID %d, error: %v", projectID, err) + return nil, fmt.Errorf("failed to get immutable tag rule by projectID %d, error: %w", projectID, err) } return r, nil } @@ -80,7 +104,7 @@ func (i *immutableRuleDao) QueryEnabledImmutableRuleByProjectID(projectID int64) var r []model.ImmutableRule _, err := qs.All(&r) if err != nil { - return nil, fmt.Errorf("failed to get enabled immutable tag rule for by projectID %d, error: %v", projectID, err) + return nil, fmt.Errorf("failed to get enabled immutable tag rule for by projectID %d, error: %w", projectID, err) } return r, nil }