Merge pull request goharbor#15548 from zyyw/ISSUE-14831

Add stop scan a particular artifact & stop scan all
perjahn · Sep 13, 2021 · 9cb266a · 9cb266a
2 parents e7a6ee7 + e2e3bcc
commit 9cb266a
Show file tree

Hide file tree

Showing 25 changed files with 460 additions and 7 deletions.
diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml
@@ -1166,6 +1166,31 @@ paths:
           $ref: '#/responses/404'
         '500':
           $ref: '#/responses/500'
+  /projects/{project_name}/repositories/{repository_name}/artifacts/{reference}/scan/stop:
+    post:
+      summary: Cancelling a scan job for a particular artifact
+      description: Cancelling a scan job for a particular artifact
+      tags:
+        - scan
+      operationId: stopScanArtifact
+      parameters:
+        - $ref: '#/parameters/requestId'
+        - $ref: '#/parameters/projectName'
+        - $ref: '#/parameters/repositoryName'
+        - $ref: '#/parameters/reference'
+      responses:
+        '202':
+          $ref: '#/responses/202'
+        '400':
+          $ref: '#/responses/400'
+        '401':
+          $ref: '#/responses/401'
+        '403':
+          $ref: '#/responses/403'
+        '404':
+          $ref: '#/responses/404'
+        '500':
+          $ref: '#/responses/500'
   /projects/{project_name}/repositories/{repository_name}/artifacts/{reference}/scan/{report_id}/log:
     get:
       summary: Get the log of the scan report
@@ -4211,6 +4236,26 @@ paths:
           $ref: '#/responses/412'
         '500':
           $ref: '#/responses/500'
+  /system/scanAll/stop:
+    post:
+      summary: Stop scanAll job execution
+      description: Stop scanAll job execution
+      parameters:
+        - $ref: '#/parameters/requestId'
+      tags:
+        - scanAll
+      operationId: stopScanAll
+      responses:
+        '202':
+          $ref: '#/responses/202'
+        '400':
+          $ref: '#/responses/400'
+        '401':
+          $ref: '#/responses/401'
+        '403':
+          $ref: '#/responses/403'
+        '500':
+          $ref: '#/responses/500'
   /ping:
     get:
       operationId: getPing

diff --git a/src/common/rbac/const.go b/src/common/rbac/const.go
@@ -30,6 +30,7 @@ const (
 
 	ActionOperate     = Action("operate")
 	ActionScannerPull = Action("scanner-pull") // for robot account created by scanner to pull image, bypass the policy check
+	ActionStop        = Action("stop")         // for stop scan/scan-all execution
 )
 
 // const resource variables

diff --git a/src/common/rbac/project/rbac_role.go b/src/common/rbac/project/rbac_role.go
@@ -98,6 +98,7 @@ var (
 
 			{Resource: rbac.ResourceScan, Action: rbac.ActionCreate},
 			{Resource: rbac.ResourceScan, Action: rbac.ActionRead},
+			{Resource: rbac.ResourceScan, Action: rbac.ActionStop},
 
 			{Resource: rbac.ResourceScanner, Action: rbac.ActionRead},
 			{Resource: rbac.ResourceScanner, Action: rbac.ActionCreate},
@@ -185,6 +186,7 @@ var (
 
 			{Resource: rbac.ResourceScan, Action: rbac.ActionCreate},
 			{Resource: rbac.ResourceScan, Action: rbac.ActionRead},
+			{Resource: rbac.ResourceScan, Action: rbac.ActionStop},
 
 			{Resource: rbac.ResourceScanner, Action: rbac.ActionRead},
 

diff --git a/src/common/rbac/system/policies.go b/src/common/rbac/system/policies.go
@@ -58,6 +58,7 @@ var (
 		{Resource: rbac.ResourceScanAll, Action: rbac.ActionUpdate},
 		{Resource: rbac.ResourceScanAll, Action: rbac.ActionDelete},
 		{Resource: rbac.ResourceScanAll, Action: rbac.ActionList},
+		{Resource: rbac.ResourceScanAll, Action: rbac.ActionStop},
 
 		{Resource: rbac.ResourceSystemVolumes, Action: rbac.ActionRead},
 

diff --git a/src/controller/event/handler/init.go b/src/controller/event/handler/init.go
@@ -30,6 +30,7 @@ func init() {
 	notifier.Subscribe(event.TopicQuotaExceed, &quota.Handler{})
 	notifier.Subscribe(event.TopicQuotaWarning, &quota.Handler{})
 	notifier.Subscribe(event.TopicScanningFailed, &scan.Handler{})
+	notifier.Subscribe(event.TopicScanningStopped, &scan.Handler{})
 	notifier.Subscribe(event.TopicScanningCompleted, &scan.Handler{})
 	notifier.Subscribe(event.TopicDeleteArtifact, &scan.DelArtHandler{})
 	notifier.Subscribe(event.TopicReplication, &artifact.ReplicationHandler{})

diff --git a/src/controller/event/metadata/scan.go b/src/controller/event/metadata/scan.go
@@ -29,7 +29,10 @@ func (si *ScanImageMetaData) Resolve(evt *event.Event) error {
 	case job.SuccessStatus:
 		eventType = event2.TopicScanningCompleted
 		topic = event2.TopicScanningCompleted
-	case job.ErrorStatus, job.StoppedStatus:
+	case job.StoppedStatus:
+		eventType = event2.TopicScanningStopped
+		topic = event2.TopicScanningStopped
+	case job.ErrorStatus:
 		eventType = event2.TopicScanningFailed
 		topic = event2.TopicScanningFailed
 	default:

diff --git a/src/controller/event/metadata/scan_test.go b/src/controller/event/metadata/scan_test.go
@@ -49,6 +49,48 @@ func (r *scanEventTestSuite) TestResolveOfScanImageEventMetadata() {
 	r.Equal("library/hello-world", data.Artifact.Repository)
 }
 
+func (r *scanEventTestSuite) TestResolveOfStopScanImageEventMetadata() {
+	e := &event.Event{}
+	metadata := &ScanImageMetaData{
+		Artifact: &v1.Artifact{
+			NamespaceID: 0,
+			Repository:  "library/hello-world",
+			Tag:         "latest",
+			Digest:      "sha256:absdfd87123",
+			MimeType:    "docker.chart",
+		},
+		Status: job.StoppedStatus.String(),
+	}
+	err := metadata.Resolve(e)
+	r.Require().Nil(err)
+	r.Equal(event2.TopicScanningStopped, e.Topic)
+	r.Require().NotNil(e.Data)
+	data, ok := e.Data.(*event2.ScanImageEvent)
+	r.Require().True(ok)
+	r.Equal("library/hello-world", data.Artifact.Repository)
+}
+
+func (r *scanEventTestSuite) TestResolveOfFailedScanImageEventMetadata() {
+	e := &event.Event{}
+	metadata := &ScanImageMetaData{
+		Artifact: &v1.Artifact{
+			NamespaceID: 0,
+			Repository:  "library/hello-world",
+			Tag:         "latest",
+			Digest:      "sha256:absdfd87123",
+			MimeType:    "docker.chart",
+		},
+		Status: job.ErrorStatus.String(),
+	}
+	err := metadata.Resolve(e)
+	r.Require().Nil(err)
+	r.Equal(event2.TopicScanningFailed, e.Topic)
+	r.Require().NotNil(e.Data)
+	data, ok := e.Data.(*event2.ScanImageEvent)
+	r.Require().True(ok)
+	r.Equal("library/hello-world", data.Artifact.Repository)
+}
+
 func TestScanEventTestSuite(t *testing.T) {
 	suite.Run(t, &scanEventTestSuite{})
 }
diff --git a/src/controller/event/topic.go b/src/controller/event/topic.go
@@ -38,6 +38,7 @@ const (
 	TopicCreateTag         = "CREATE_TAG"
 	TopicDeleteTag         = "DELETE_TAG"
 	TopicScanningFailed    = "SCANNING_FAILED"
+	TopicScanningStopped   = "SCANNING_STOPPED"
 	TopicScanningCompleted = "SCANNING_COMPLETED"
 	// QuotaExceedTopic is topic for quota warning event, the usage reaches the warning bar of limitation, like 85%
 	TopicQuotaWarning    = "QUOTA_WARNING"

diff --git a/src/controller/scan/base_controller.go b/src/controller/scan/base_controller.go
@@ -313,6 +313,24 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti
 	return nil
 }
 
+// Stop scan job of a given artifact
+func (bc *basicController) Stop(ctx context.Context, artifact *ar.Artifact) error {
+	if artifact == nil {
+		return errors.New("nil artifact to stop scan")
+	}
+	query := q.New(q.KeyWords{"extra_attrs.artifact.digest": artifact.Digest})
+	executions, err := bc.execMgr.List(ctx, query)
+	if err != nil {
+		return err
+	}
+	if len(executions) == 0 {
+		message := fmt.Sprintf("no scan job for artifact digest=%v", artifact.Digest)
+		return errors.BadRequestError(nil).WithMessage(message)
+	}
+	execution := executions[0]
+	return bc.execMgr.Stop(ctx, execution.ID)
+}
+
 func (bc *basicController) ScanAll(ctx context.Context, trigger string, async bool) (int64, error) {
 	executionID, err := bc.execMgr.Create(ctx, VendorTypeScanAll, 0, trigger)
 	if err != nil {

diff --git a/src/controller/scan/base_controller_test.go b/src/controller/scan/base_controller_test.go
@@ -358,6 +358,45 @@ func (suite *ControllerTestSuite) TestScanControllerScan() {
 	}
 }
 
+// TestScanControllerStop ...
+func (suite *ControllerTestSuite) TestScanControllerStop() {
+	{
+		// artifact not provieded
+		suite.Require().Error(suite.c.Stop(context.TODO(), nil))
+	}
+
+	{
+		// success
+		mock.OnAnything(suite.execMgr, "List").Return([]*task.Execution{
+			{ExtraAttrs: suite.makeExtraAttrs("rp-uuid-001"), Status: "Running"},
+		}, nil).Once()
+		mock.OnAnything(suite.execMgr, "Stop").Return(nil).Once()
+
+		ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
+
+		suite.Require().NoError(suite.c.Stop(ctx, suite.artifact))
+	}
+
+	{
+		// failed due to no execution returned by List
+		mock.OnAnything(suite.execMgr, "List").Return([]*task.Execution{}, nil).Once()
+		mock.OnAnything(suite.execMgr, "Stop").Return(nil).Once()
+
+		ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
+
+		suite.Require().Error(suite.c.Stop(ctx, suite.artifact))
+	}
+
+	{
+		// failed due to execMgr.List() errored out
+		mock.OnAnything(suite.execMgr, "List").Return([]*task.Execution{}, fmt.Errorf("failed to call execMgr.List()")).Once()
+
+		ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{})
+
+		suite.Require().Error(suite.c.Stop(ctx, suite.artifact))
+	}
+}
+
 // TestScanControllerGetReport ...
 func (suite *ControllerTestSuite) TestScanControllerGetReport() {
 	mock.OnAnything(suite.ar, "Walk").Return(nil).Run(func(args mock.Arguments) {

diff --git a/src/controller/scan/controller.go b/src/controller/scan/controller.go
@@ -50,6 +50,16 @@ type Controller interface {
 	//     error  : non nil error if any errors occurred
 	Scan(ctx context.Context, artifact *artifact.Artifact, options ...Option) error
 
+	// Stop scan job of the given artifact
+	//
+	//   Arguments:
+	//     ctx context.Context : the context for this method
+	//     artifact *artifact.Artifact : the artifact whose scan job to be stopped
+	//
+	//   Returns:
+	//     error  : non nil error if any errors occurred
+	Stop(ctx context.Context, artifact *artifact.Artifact) error
+
 	// GetReport gets the reports for the given artifact identified by the digest
 	//
 	//   Arguments:

diff --git a/src/pkg/notification/notification.go b/src/pkg/notification/notification.go
@@ -58,6 +58,7 @@ func initSupportedNotifyType() {
 		event.TopicQuotaExceed,
 		event.TopicQuotaWarning,
 		event.TopicScanningFailed,
+		event.TopicScanningStopped,
 		event.TopicScanningCompleted,
 		event.TopicReplication,
 		event.TopicTagRetention,

diff --git a/src/pkg/scan/job.go b/src/pkg/scan/job.go
@@ -146,6 +146,16 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error {
 	// Get logger
 	myLogger := ctx.GetLogger()
 
+	// shouldStop checks if the job should be stopped
+	shouldStop := func() bool {
+		if cmd, ok := ctx.OPCommand(); ok && cmd == job.StopCommand {
+			myLogger.Info("scan job being stopped")
+			return true
+		}
+
+		return false
+	}
+
 	// Ignore errors as they have been validated already
 	r, _ := extractRegistration(params)
 	req, _ := ExtractScanReq(params)
@@ -156,6 +166,10 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error {
 	printJSONParameter(JobParameterRequest, removeAuthInfo(req), myLogger)
 	myLogger.Infof("Report mime types: %v\n", mimeTypes)
 
+	if shouldStop() {
+		return nil
+	}
+
 	// Submit scan request to the scanner adapter
 	client, err := r.Client(v1.DefaultClientPool)
 	if err != nil {
@@ -182,6 +196,10 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error {
 		logAndWrapError(myLogger, err, "scan job: make authorization")
 	}
 
+	if shouldStop() {
+		return nil
+	}
+
 	req.Registry.Authorization = authorization
 	resp, err := client.SubmitScan(req)
 	if err != nil {
@@ -210,6 +228,10 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error {
 			for {
 				select {
 				case t := <-tm.C:
+					if shouldStop() {
+						return
+					}
+
 					myLogger.Debugf("check scan report for mime %s at %s", m, t.Format("2006/01/02 15:04:05"))
 
 					rawReport, err := client.GetScanReport(resp.ID, m)
@@ -250,6 +272,10 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error {
 	// Wait for all the retrieving routines are completed
 	wg.Wait()
 
+	if shouldStop() {
+		return nil
+	}
+
 	// Merge errors
 	for _, e := range errs {
 		if e != nil {

diff --git a/src/pkg/scan/job_test.go b/src/pkg/scan/job_test.go
@@ -65,6 +65,7 @@ func (suite *JobTestSuite) TestJob() {
 	lg := &mockjobservice.MockJobLogger{}
 
 	ctx.On("GetLogger").Return(lg)
+	ctx.On("OPCommand").Return(job.NilCommand, false)
 
 	r := &scanner.Registration{
 		ID:   0,

diff --git a/src/pkg/scan/report/summary.go b/src/pkg/scan/report/summary.go
@@ -113,9 +113,8 @@ func GenerateNativeSummary(r *scan.Report, options ...Option) (interface{}, erro
 
 	sum.TotalCount = 1
 
-	// If the status is not success/stopped, there will not be any report.
-	if r.Status != job.SuccessStatus.String() &&
-		r.Status != job.StoppedStatus.String() {
+	// If the status is not success, there will not be any report.
+	if r.Status != job.SuccessStatus.String() {
 		return sum, nil
 	}
 

diff --git a/src/pkg/scan/vuln/util.go b/src/pkg/scan/vuln/util.go
@@ -85,6 +85,8 @@ func MergeScanStatus(s1, s2 string) string {
 
 	if j1 == job.RunningStatus || j2 == job.RunningStatus {
 		return job.RunningStatus.String()
+	} else if j1 == job.StoppedStatus || j2 == job.StoppedStatus {
+		return job.StoppedStatus.String()
 	} else if j1 == job.SuccessStatus || j2 == job.SuccessStatus {
 		// the scan status of the image index will be treated as a success when one of its children is success
 		return job.SuccessStatus.String()

diff --git a/src/pkg/scan/vuln/util_test.go b/src/pkg/scan/vuln/util_test.go
@@ -71,6 +71,7 @@ func Test_mergeScanStatus(t *testing.T) {
 	errorStatus := job.ErrorStatus.String()
 	runningStatus := job.RunningStatus.String()
 	successStatus := job.SuccessStatus.String()
+	stoppedStatus := job.StoppedStatus.String()
 
 	type args struct {
 		s1 string
@@ -88,6 +89,9 @@ func Test_mergeScanStatus(t *testing.T) {
 		{"success and success", args{successStatus, successStatus}, successStatus},
 		{"error and error", args{errorStatus, errorStatus}, errorStatus},
 		{"error and empty string", args{errorStatus, ""}, errorStatus},
+		{"running and stopped", args{runningStatus, stoppedStatus}, runningStatus},
+		{"success and stopped", args{successStatus, stoppedStatus}, stoppedStatus},
+		{"stopped and stopped", args{stoppedStatus, stoppedStatus}, stoppedStatus},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {