return more clear error message for scan related API

- add a common error pkg to support error with code and AsError check
- replace some errors in scan with coded errors
- fix #9668

Signed-off-by: Steven Zou <szou@vmware.com>
This commit is contained in:
Steven Zou 2019-10-31 11:33:43 +08:00
parent 5d6cbe9aa1
commit afb46188b2
4 changed files with 176 additions and 7 deletions

View File

@ -18,14 +18,14 @@ import (
"net/http"
"strconv"
"github.com/goharbor/harbor/src/pkg/scan/report"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/common/utils"
coreutils "github.com/goharbor/harbor/src/core/utils"
"github.com/goharbor/harbor/src/jobservice/logger"
"github.com/goharbor/harbor/src/pkg/errs"
"github.com/goharbor/harbor/src/pkg/scan/api/scan"
"github.com/goharbor/harbor/src/pkg/scan/report"
v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
"github.com/pkg/errors"
)
@ -94,7 +94,19 @@ func (sa *ScanAPI) Scan() {
}
if err := scan.DefaultController.Scan(sa.artifact); err != nil {
sa.SendInternalServerError(errors.Wrap(err, "scan API: scan"))
e := errors.Wrap(err, "scan API: scan")
if errs.AsError(err, errs.PreconditionFailed) {
sa.SendPreconditionFailedError(e)
return
}
if errs.AsError(err, errs.Conflict) {
sa.SendConflictError(e)
return
}
sa.SendInternalServerError(e)
return
}
@ -117,7 +129,14 @@ func (sa *ScanAPI) Report() {
// Get the reports
reports, err := scan.DefaultController.GetReport(sa.artifact, producesMimes)
if err != nil {
sa.SendInternalServerError(errors.Wrap(err, "scan API: get report"))
e := errors.Wrap(err, "scan API: get report")
if errs.AsError(err, errs.PreconditionFailed) {
sa.SendPreconditionFailedError(e)
return
}
sa.SendInternalServerError(e)
return
}

132
src/pkg/errs/error.go Normal file
View File

@ -0,0 +1,132 @@
// 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 errs
import (
"encoding/json"
"fmt"
)
const (
// Common error code
Common uint16 = 10000
// Conflict error code
Conflict uint16 = 10409
// PreconditionFailed error code
PreconditionFailed uint16 = 10412
)
// codeTexts behaviors as a hash map to look for the text for the given error code.
func codeTexts(code uint16) string {
switch code {
case Common:
return "common"
case Conflict:
return "not found"
case PreconditionFailed:
return "Precondition failed"
default:
return "unknown"
}
}
// Error with code
type Error struct {
// Code of error
Code uint16 `json:"code"`
// Code represented by meaningful text
TextCode string `json:"text_code"`
// Message of error
Message string `json:"message"`
// Cause for error
Cause error `json:"cause"`
}
// Error message
func (e *Error) Error() string {
emsg := fmt.Sprintf("error: code %d:%s : %s", e.Code, e.TextCode, e.Message)
if e.Cause != nil {
emsg = fmt.Sprintf("%s : cause: %s", emsg, e.Cause.Error())
}
return emsg
}
// String outputs the error with well-formatted string.
func (e *Error) String() string {
bytes, err := json.Marshal(e)
if err != nil {
// Fallback to normal string
return e.Error()
}
return string(bytes)
}
// New common error.
func New(message string) error {
return &Error{
Code: Common,
TextCode: codeTexts(Common),
Message: message,
}
}
// Wrap error with message.
func Wrap(err error, message string) error {
return &Error{
Code: Common,
TextCode: codeTexts(Common),
Message: message,
Cause: err,
}
}
// Errorf new a message with the specified format and arguments
func Errorf(format string, args ...interface{}) error {
return &Error{
Code: Common,
TextCode: codeTexts(Common),
Message: fmt.Sprintf(format, args...),
}
}
// WithCode sets specified code for the error
func WithCode(code uint16, err error) error {
if err == nil {
return err
}
e, ok := err.(*Error)
if !ok {
return err
}
e.Code = code
e.TextCode = codeTexts(code)
return e
}
// AsError checks if the given error has the given code
func AsError(err error, code uint16) bool {
if err == nil {
return false
}
e, ok := err.(*Error)
return ok && e.Code == code
}

View File

@ -28,6 +28,7 @@ import (
"github.com/goharbor/harbor/src/core/service/token"
"github.com/goharbor/harbor/src/jobservice/job"
"github.com/goharbor/harbor/src/jobservice/logger"
"github.com/goharbor/harbor/src/pkg/errs"
"github.com/goharbor/harbor/src/pkg/robot"
"github.com/goharbor/harbor/src/pkg/robot/model"
sca "github.com/goharbor/harbor/src/pkg/scan"
@ -124,9 +125,14 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error {
return errors.Wrap(err, "scan controller: scan")
}
// In case it does not exist
if r == nil {
return errs.WithCode(errs.PreconditionFailed, errs.Errorf("no available scanner for project: %s", artifact.NamespaceID))
}
// Check if it is disabled
if r.Disabled {
return errors.Errorf("scanner %s is disabled", r.Name)
return errs.WithCode(errs.PreconditionFailed, errs.Errorf("scanner %s is disabled", r.Name))
}
// Check the health of the registration by ping.
@ -145,6 +151,7 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error {
producesMimes := make([]string, 0)
matched := false
statusConflict := false
for _, ca := range meta.Capabilities {
for _, cm := range ca.ConsumesMimeTypes {
if cm == artifact.MimeType {
@ -166,6 +173,12 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error {
}
_, e := bc.manager.Create(reportPlaceholder)
if e != nil {
// Check if it is a status conflict error with common error format.
// Common error returned if and only if status conflicts.
if !statusConflict {
statusConflict = errs.AsError(e, errs.Conflict)
}
// Recorded by error wrap and logged at the same time.
if err == nil {
err = e
@ -192,6 +205,10 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error {
// If all the record are created failed.
if len(producesMimes) == 0 {
// Return the last error
if statusConflict {
return errs.WithCode(errs.Conflict, errs.Wrap(err, "scan controller: scan"))
}
return errors.Wrap(err, "scan controller: scan")
}
@ -241,7 +258,7 @@ func (bc *basicController) GetReport(artifact *v1.Artifact, mimeTypes []string)
}
if r == nil {
return nil, errors.New("no scanner registration configured")
return nil, errs.WithCode(errs.PreconditionFailed, errs.Errorf("no scanner registration configured for project: %d", artifact.NamespaceID))
}
return bc.manager.GetBy(artifact.Digest, r.UUID, mimes)

View File

@ -18,6 +18,7 @@ import (
"time"
"github.com/goharbor/harbor/src/jobservice/job"
"github.com/goharbor/harbor/src/pkg/errs"
"github.com/goharbor/harbor/src/pkg/q"
"github.com/goharbor/harbor/src/pkg/scan/dao/scan"
"github.com/google/uuid"
@ -78,7 +79,7 @@ func (bm *basicManager) Create(r *scan.Report) (string, error) {
// Status conflict
if theCopy.StartTime.Add(reportTimeout).After(time.Now()) {
if theStatus.Compare(job.RunningStatus) <= 0 {
return "", errors.Errorf("conflict: a previous scanning is %s", theCopy.Status)
return "", errs.WithCode(errs.Conflict, errs.Errorf("a previous scan process is %s", theCopy.Status))
}
}