Skip to content

Commit

Permalink
Merge pull request goharbor#9673 from steven-zou/fix/issue_#9668_stat…
Browse files Browse the repository at this point in the history
…us_conflicts

return more clear error message for scan related API
  • Loading branch information
reasonerjt authored Nov 1, 2019
2 parents 2101d71 + eb8ec49 commit f2beee1
Show file tree
Hide file tree
Showing 5 changed files with 294 additions and 7 deletions.
27 changes: 23 additions & 4 deletions src/core/api/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/scan/api/scan"
"github.com/goharbor/harbor/src/pkg/scan/errs"
"github.com/goharbor/harbor/src/pkg/scan/report"
v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
21 changes: 19 additions & 2 deletions src/pkg/scan/api/scan/base_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
sc "github.com/goharbor/harbor/src/pkg/scan/api/scanner"
"github.com/goharbor/harbor/src/pkg/scan/dao/scan"
"github.com/goharbor/harbor/src/pkg/scan/dao/scanner"
"github.com/goharbor/harbor/src/pkg/scan/errs"
"github.com/goharbor/harbor/src/pkg/scan/report"
v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
"github.com/google/uuid"
Expand Down Expand Up @@ -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: %d", 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.
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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")
}

Expand Down Expand Up @@ -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)
Expand Down
132 changes: 132 additions & 0 deletions src/pkg/scan/errs/error.go
Original file line number Diff line number Diff line change
@@ -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 "conflict"
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: %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
}
118 changes: 118 additions & 0 deletions src/pkg/scan/errs/error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// 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"
"strings"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
)

// ErrorSuite is a test suite for testing Error.
type ErrorSuite struct {
suite.Suite
}

// TestError is the entry point of ErrorSuite.
func TestError(t *testing.T) {
suite.Run(t, &ErrorSuite{})
}

// TestErrorNew ...
func (suite *ErrorSuite) TestErrorNew() {
err := New("error-testing")
require.Error(suite.T(), err)

suite.Equal(true, AsError(err, Common))
suite.Condition(func() (success bool) {
return -1 != strings.Index(err.Error(), "error-testing")
})
suite.Condition(func() (success bool) {
success = strings.Contains(err.Error(), codeTexts(Common))
return
})
}

// TestErrorWrap ...
func (suite *ErrorSuite) TestErrorWrap() {
err := errors.New("error-stack")
e := Wrap(err, "wrap-message")
require.Error(suite.T(), e)

suite.Equal(true, AsError(e, Common))
suite.Condition(func() (success bool) {
success = -1 != strings.Index(e.Error(), "error-stack")
return
})
suite.Condition(func() (success bool) {
success = -1 != strings.Index(e.Error(), "wrap-message")
return
})
suite.Condition(func() (success bool) {
success = strings.Contains(e.Error(), codeTexts(Common))
return
})
}

// TestErrorErrorf ...
func (suite *ErrorSuite) TestErrorErrorf() {
err := Errorf("a=%d", 1000)
require.Error(suite.T(), err)

suite.Equal(true, AsError(err, Common))
suite.Condition(func() (success bool) {
success = strings.Contains(err.Error(), "a=1000")
return
})
suite.Condition(func() (success bool) {
success = strings.Contains(err.Error(), codeTexts(Common))
return
})
}

// TestErrorString ...
func (suite *ErrorSuite) TestErrorString() {
err := New("well-formatted-error")
require.Error(suite.T(), err)

str := err.(*Error).String()
require.Condition(suite.T(), func() (success bool) {
success = len(str) > 0
return
})

e := &Error{}
er := json.Unmarshal([]byte(str), e)
suite.NoError(er)
suite.Equal(e.Message, "well-formatted-error")
}

// TestErrorWithCode ...
func (suite *ErrorSuite) TestErrorWithCode() {
err := New("error-with-code")
require.Error(suite.T(), err)

err = WithCode(Conflict, err)
require.Error(suite.T(), err)
suite.Equal(true, AsError(err, Conflict))
suite.Condition(func() (success bool) {
success = strings.Contains(err.Error(), codeTexts(Conflict))
return
})
}
3 changes: 2 additions & 1 deletion src/pkg/scan/report/base_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/goharbor/harbor/src/jobservice/job"
"github.com/goharbor/harbor/src/pkg/q"
"github.com/goharbor/harbor/src/pkg/scan/dao/scan"
"github.com/goharbor/harbor/src/pkg/scan/errs"
"github.com/google/uuid"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -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))
}
}

Expand Down

0 comments on commit f2beee1

Please sign in to comment.