- fix API test cases failures

Signed-off-by: Steven Zou <szou@vmware.com>

- fix scan report dao bug

src/core/api/scan_all.go

@@ -1,14 +1,15 @@
 package api
 
 import (
-	"errors"
 	"net/http"
 	"strconv"
 
+	"github.com/goharbor/harbor/src/pkg/q"
+
 	common_job "github.com/goharbor/harbor/src/common/job"
-	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/core/api/models"
-	"github.com/goharbor/harbor/src/core/config"
+	"github.com/goharbor/harbor/src/pkg/scan/api/scanner"
+	"github.com/pkg/errors"
 )
 
 // ScanAllAPI handles request of scan all images...
@@ -19,11 +20,7 @@ type ScanAllAPI struct {
 // Prepare validates the URL and parms, it needs the system admin permission.
 func (sc *ScanAllAPI) Prepare() {
 	sc.BaseController.Prepare()
-	if !config.WithClair() {
-		log.Warningf("Harbor is not deployed with Clair, it's not possible to scan images.")
-		sc.SendStatusServiceUnavailableError(errors.New(""))
-		return
-	}
+
 	if !sc.SecurityCtx.IsAuthenticated() {
 		sc.SendUnAuthorizedError(errors.New("UnAuthorized"))
 		return
@@ -32,6 +29,17 @@ func (sc *ScanAllAPI) Prepare() {
 		sc.SendForbiddenError(errors.New(sc.SecurityCtx.GetUsername()))
 		return
 	}
+
+	enabled, err := isScanEnabled()
+	if err != nil {
+		sc.SendInternalServerError(err)
+		return
+	}
+
+	if !enabled {
+		sc.SendStatusServiceUnavailableError(errors.New("no scanner is configured, it's not possible to scan"))
+		return
+	}
 }
 
 // Post according to the request, it creates a cron schedule or a manual trigger for scan all.
@@ -88,3 +96,19 @@ func (sc *ScanAllAPI) Get() {
 func (sc *ScanAllAPI) List() {
 	sc.list(common_job.ImageScanAllJob)
 }
+
+func isScanEnabled() (bool, error) {
+	kws := make(map[string]interface{})
+	kws["is_default"] = true
+
+	query := &q.Query{
+		Keywords: kws,
+	}
+
+	l, err := scanner.DefaultController.ListRegistrations(query)
+	if err != nil {
+		return false, errors.Wrap(err, "scan all API: check if scan is enabled")
+	}
+
+	return len(l) > 0, nil
+}

src/core/api/scan_all_test.go

@@ -3,36 +3,66 @@ package api
 import (
 	"testing"
 
+	"github.com/goharbor/harbor/src/pkg/scan/dao/scanner"
+	sc "github.com/goharbor/harbor/src/pkg/scan/scanner"
 	"github.com/goharbor/harbor/src/testing/apitests/apilib"
-	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
 )
 
 var adminJob002 apilib.AdminJobReq
 
-func TestScanAllPost(t *testing.T) {
+// ScanAllAPITestSuite is a test suite to test scan all API.
+type ScanAllAPITestSuite struct {
+	suite.Suite
 
-	assert := assert.New(t)
+	m    sc.Manager
+	uuid string
+}
+
+// TestScanAllAPI is an entry point for ScanAllAPITestSuite.
+func TestScanAllAPI(t *testing.T) {
+	suite.Run(t, &ScanAllAPITestSuite{})
+}
+
+// SetupSuite prepares env for test suite.
+func (suite *ScanAllAPITestSuite) SetupSuite() {
+	// Ensure scanner is there
+	reg := &scanner.Registration{
+		Name:        "Clair",
+		Description: "The clair scanner adapter",
+		URL:         "https://clair.com:8080",
+		Disabled:    false,
+		IsDefault:   true,
+	}
+
+	scMgr := sc.New()
+	uuid, err := scMgr.Create(reg)
+	require.NoError(suite.T(), err, "failed to initialize clair scanner")
+
+	suite.uuid = uuid
+	suite.m = scMgr
+}
+
+// TearDownSuite clears env for the test suite.
+func (suite *ScanAllAPITestSuite) TearDownSuite() {
+	err := suite.m.Delete(suite.uuid)
+	suite.NoError(err, "clear scanner")
+}
+
+func (suite *ScanAllAPITestSuite) TestScanAllPost() {
 	apiTest := newHarborAPI()
 
 	// case 1: add a new scan all job
 	code, err := apiTest.AddScanAll(*admin, adminJob002)
-	if err != nil {
-		t.Error("Error occurred while add a scan all job", err.Error())
-		t.Log(err)
-	} else {
-		assert.Equal(200, code, "Add scan all status should be 200")
-	}
+	require.NoError(suite.T(), err, "Error occurred while add a scan all job")
+	suite.Equal(200, code, "Add scan all status should be 200")
 }
 
-func TestScanAllGet(t *testing.T) {
-	assert := assert.New(t)
+func (suite *ScanAllAPITestSuite) TestScanAllGet() {
 	apiTest := newHarborAPI()
 
 	code, _, err := apiTest.ScanAllScheduleGet(*admin)
-	if err != nil {
-		t.Error("Error occurred while get a scan all job", err.Error())
-		t.Log(err)
-	} else {
-		assert.Equal(200, code, "Get scan all status should be 200")
-	}
+	require.NoError(suite.T(), err, "Error occurred while get a scan all job")
+	suite.Equal(200, code, "Get scan all status should be 200")
 }

src/pkg/scan/api/scan/base_controller.go

@@ -119,6 +119,11 @@ func (bc *basicController) Scan(artifact *v1.Artifact) error {
 		return errors.Wrap(err, "scan controller: scan")
 	}
 
+	// Check if it is disabled
+	if r.Disabled {
+		return errors.Errorf("scanner %s is disabled", r.Name)
+	}
+
 	// Check the health of the registration by ping.
 	// The metadata of the scanner adapter is also returned.
 	meta, err := bc.sc.Ping(r)
@@ -296,9 +301,8 @@ func (bc *basicController) HandleJobHooks(trackID string, change *job.StatusChan
 	}
 
 	// Clear robot account
-	// All final statuses (success, error and stopped) share the same code.
-	// Only need to check one of them.
-	if job.Status(change.Status).Compare(job.SuccessStatus) == 0 {
+	// Only when the job is successfully done!
+	if change.Status == job.SuccessStatus.String() {
 		if v, ok := change.Metadata.Parameters[sca.JobParameterRobotID]; ok {
 			if rid, y := v.(float64); y {
 				if err := robot.RobotCtr.DeleteRobotAccount(int64(rid)); err != nil {

src/pkg/scan/dao/scan/report.go

@@ -63,6 +63,7 @@ func ListReports(query *q.Query) ([]*Report, error) {
 			for k, v := range query.Keywords {
 				if vv, ok := v.([]interface{}); ok {
 					qt = qt.Filter(fmt.Sprintf("%s__in", k), vv...)
+					continue
 				}
 
 				qt = qt.Filter(k, v)

src/pkg/scan/report/base_manager.go

@@ -24,6 +24,10 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	reportTimeout = 1 * time.Hour
+)
+
 // basicManager is a default implementation of report manager.
 type basicManager struct{}
 
@@ -72,8 +76,10 @@ func (bm *basicManager) Create(r *scan.Report) (string, error) {
 		}
 
 		// Status conflict
-		if theStatus.Compare(job.RunningStatus) <= 0 {
-			return "", errors.Errorf("conflict: a previous scanning is %s", theCopy.Status)
+		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)
+			}
 		}
 
 		// Otherwise it will be a completed report
@@ -201,7 +207,13 @@ func (bm *basicManager) DeleteByDigests(digests ...string) error {
 	}
 
 	kws := make(map[string]interface{})
-	kws["digest"] = digests
+	ds := make([]interface{}, 0)
+
+	for _, dig := range digests {
+		ds = append(ds, dig)
+	}
+
+	kws["digest"] = ds
 	query := &q.Query{
 		Keywords: kws,
 	}

src/pkg/scan/report/summary.go

@@ -82,6 +82,9 @@ func GenerateNativeSummary(r *scan.Report, options ...Option) (interface{}, erro
 	sum.StartTime = r.StartTime
 	sum.EndTime = r.EndTime
 	sum.Duration = r.EndTime.Unix() - r.StartTime.Unix()
+	if sum.Duration < 0 {
+		sum.Duration = 0
+	}
 	if len(ops.CVEWhitelist) > 0 {
 		sum.CVEBypassed = make([]string, 0)
 	}