add OCI error format support

1, Leverage go v1.13 new error feature
2, Define genernal error OCI format, so that /v2 API could return a OCI compatible error

Signed-off-by: wang yan <wangyan@vmware.com>

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": <error identifier>,
+//			"message": <message describing condition>,
+//			// optional
+//			"detail": <unstructured>
+//		},
+//		...
+//	]
+// }
+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]
+}

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

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
 		}

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.True(t, IsDupRecErr(fmt.Errorf("pq: duplicate key value violates unique constraint \"properties_k_key\"")))
-	assert.False(t, isDupRecErr(fmt.Errorf("other error")))
+	assert.False(t, IsDupRecErr(fmt.Errorf("other error")))
 }
 
 func TestWithTransaction(t *testing.T) {

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 {
+		if errors.Is(err, errNotFound) {
-			b.SendNotFoundError(fmt.Errorf("project %v not found", projectIDOrName))
+			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

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() {
+	// Check access permissions
-		itr.SendUnAuthorizedError(errors.New("Unauthorized"))
+	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
 	}
 }

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)),
+		pkgE := internal_errors.New(fmt.Errorf("error occurred when to handle request in immutable handler: %v", err)).WithCode(internal_errors.GeneralCode)
-			http.StatusInternalServerError)
+		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 {
+		var e *middlerware_err.ErrImmutable
-			http.Error(rw, util.MarshalError("DENIED",
+		if errors.As(err, &e) {
-				fmt.Sprintf("%v", err)), http.StatusPreconditionFailed)
+			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
 	}
 

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 {
+func NewErrImmutable(msg, tag string) error {
-	return ErrImmutable{
+	return &ErrImmutable{
 		repo: msg,
 		tag:  tag,
 	}

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

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
 }