Fix merge scan summary (#11392)

* fix(scan): fix ScanStatus when merge NativeReportSummary

1. Running and success status is high priority when merge ScanStatus of
NativeReportSummary, otherwise chose the bigger status.
2. Merge scan logs of referenced artifacts when get the scan logs of
image index.

Closes #11265

Signed-off-by: He Weiwei <hweiwei@vmware.com>

* fix(portal): fix the annotation for the scan completed percent in scan overview

Signed-off-by: He Weiwei <hweiwei@vmware.com>

src/controller/scan/base_controller.go

@@ -15,9 +15,11 @@
 package scan
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"fmt"
+	"strings"
 	"sync"
 
 	cj "github.com/goharbor/harbor/src/common/job"
@@ -39,6 +41,7 @@ import (
 	"github.com/goharbor/harbor/src/pkg/scan/dao/scanner"
 	"github.com/goharbor/harbor/src/pkg/scan/report"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
+	"github.com/goharbor/harbor/src/pkg/scan/vuln"
 	"github.com/google/uuid"
 )
 
@@ -359,8 +362,7 @@ func (bc *basicController) GetReport(ctx context.Context, artifact *ar.Artifact,
 		if len(group) != 0 {
 			reports = append(reports, group...)
 		} else {
-			// NOTE: If the artifact is OCI image, this happened when the artifact is not scanned.
-			// If the artifact is OCI image index, this happened when the artifact is not scanned,
+			// NOTE: If the artifact is OCI image, this happened when the artifact is not scanned,
 			// but its children artifacts may scanned so return empty report
 			return nil, nil
 		}
@@ -403,12 +405,7 @@ func (bc *basicController) GetSummary(ctx context.Context, artifact *ar.Artifact
 	return summaries, nil
 }
 
-// GetScanLog ...
-func (bc *basicController) GetScanLog(uuid string) ([]byte, error) {
-	if len(uuid) == 0 {
-		return nil, errors.New("empty uuid to get scan log")
-	}
-
+func (bc *basicController) getScanLog(uuid string) ([]byte, error) {
 	// Get by uuid
 	sr, err := bc.manager.Get(uuid)
 	if err != nil {
@@ -432,6 +429,78 @@ func (bc *basicController) GetScanLog(uuid string) ([]byte, error) {
 	return bc.jc().GetJobLog(sr.JobID)
 }
 
+// GetScanLog ...
+func (bc *basicController) GetScanLog(uuid string) ([]byte, error) {
+	if len(uuid) == 0 {
+		return nil, errors.New("empty uuid to get scan log")
+	}
+
+	data, err := base64.StdEncoding.DecodeString(uuid)
+	if err != nil {
+		data = []byte(uuid)
+	}
+
+	reportIDs := strings.Split(string(data), vuln.SummaryReportIDSeparator)
+
+	errs := map[string]error{}
+	logs := make(map[string][]byte, len(reportIDs))
+
+	var (
+		mu sync.Mutex
+		wg sync.WaitGroup
+	)
+	for _, reportID := range reportIDs {
+		wg.Add(1)
+
+		go func(reportID string) {
+			defer wg.Done()
+
+			log, err := bc.getScanLog(reportID)
+
+			mu.Lock()
+			defer mu.Unlock()
+
+			if err != nil {
+				errs[reportID] = err
+			} else {
+				logs[reportID] = log
+			}
+		}(reportID)
+	}
+	wg.Wait()
+
+	if len(reportIDs) == 1 {
+		return logs[reportIDs[0]], errs[reportIDs[0]]
+	}
+
+	if len(errs) == len(reportIDs) {
+		for _, err := range errs {
+			return nil, err
+		}
+	}
+
+	var b bytes.Buffer
+
+	multiLogs := len(logs) > 1
+	for _, reportID := range reportIDs {
+		log, ok := logs[reportID]
+		if !ok || len(log) == 0 {
+			continue
+		}
+
+		if multiLogs {
+			if b.Len() > 0 {
+				b.WriteString("\n\n\n\n")
+			}
+			b.WriteString(fmt.Sprintf("---------- Logs of report %s ----------\n", reportID))
+		}
+
+		b.Write(log)
+	}
+
+	return b.Bytes(), nil
+}
+
 // HandleJobHooks ...
 func (bc *basicController) HandleJobHooks(trackID string, change *job.StatusChange) error {
 	if len(trackID) == 0 {

src/controller/scan/base_controller_test.go

@@ -53,8 +53,9 @@ type ControllerTestSuite struct {
 	artifact     *artifact.Artifact
 	rawReport    string
 
-	ar artifact.Controller
-	c  Controller
+	reportMgr *reporttesting.Manager
+	ar        artifact.Controller
+	c         Controller
 }
 
 // TestController is the entry point of ControllerTestSuite.
@@ -163,6 +164,7 @@ func (suite *ControllerTestSuite) SetupSuite() {
 	mgr.On("Get", "rp-uuid-001").Return(reports[0], nil)
 	mgr.On("UpdateReportData", "rp-uuid-001", suite.rawReport, (int64)(10000)).Return(nil)
 	mgr.On("UpdateStatus", "the-uuid-123", "Success", (int64)(10000)).Return(nil)
+	suite.reportMgr = mgr
 
 	rc := &MockRobotController{}
 
@@ -288,6 +290,60 @@ func (suite *ControllerTestSuite) TestScanControllerGetScanLog() {
 	})
 }
 
+func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() {
+	{
+		// Both success
+		suite.reportMgr.On("Get", "rp-uuid-002").Return(&scan.Report{
+			ID:               12,
+			UUID:             "rp-uuid-002",
+			Digest:           "digest-code",
+			RegistrationUUID: "uuid001",
+			MimeType:         "application/vnd.scanner.adapter.vuln.report.harbor+json; version=1.0",
+			Status:           "Success",
+			StatusCode:       3,
+			TrackID:          "the-uuid-124",
+			JobID:            "the-job-id",
+			StatusRevision:   time.Now().Unix(),
+			Report:           suite.rawReport,
+			StartTime:        time.Now(),
+			EndTime:          time.Now().Add(2 * time.Second),
+		}, nil).Once()
+
+		bytes, err := suite.c.GetScanLog(base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
+		suite.Nil(err)
+		suite.NotEmpty(bytes)
+		suite.Contains(string(bytes), "Logs of report rp-uuid-001")
+		suite.Contains(string(bytes), "Logs of report rp-uuid-002")
+	}
+
+	{
+		// One successfully, one failed
+		suite.reportMgr.On("Get", "rp-uuid-002").Return(nil, fmt.Errorf("error")).Once()
+		bytes, err := suite.c.GetScanLog(base64.StdEncoding.EncodeToString([]byte("rp-uuid-001|rp-uuid-002")))
+		suite.Nil(err)
+		suite.NotEmpty(bytes)
+		suite.NotContains(string(bytes), "Logs of report rp-uuid-001")
+	}
+
+	{
+		// Both failed
+		suite.reportMgr.On("Get", "rp-uuid-002").Return(nil, fmt.Errorf("error")).Once()
+		suite.reportMgr.On("Get", "rp-uuid-003").Return(nil, fmt.Errorf("error")).Once()
+		bytes, err := suite.c.GetScanLog(base64.StdEncoding.EncodeToString([]byte("rp-uuid-002|rp-uuid-003")))
+		suite.Error(err)
+		suite.Empty(bytes)
+	}
+
+	{
+		// Both empty
+		suite.reportMgr.On("Get", "rp-uuid-002").Return(nil, nil).Once()
+		suite.reportMgr.On("Get", "rp-uuid-003").Return(nil, nil).Once()
+		bytes, err := suite.c.GetScanLog(base64.StdEncoding.EncodeToString([]byte("rp-uuid-002|rp-uuid-003")))
+		suite.Nil(err)
+		suite.Empty(bytes)
+	}
+}
+
 // TestScanControllerHandleJobHooks ...
 func (suite *ControllerTestSuite) TestScanControllerHandleJobHooks() {
 	cReport := &sca.CheckInReport{

src/pkg/scan/vuln/summary.go

@@ -15,12 +15,18 @@
 package vuln
 
 import (
+	"encoding/base64"
 	"time"
 
 	"github.com/goharbor/harbor/src/jobservice/job"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
 )
 
+const (
+	// SummaryReportIDSeparator the separator of the ReportID in the summary when its merged by multi summaries
+	SummaryReportIDSeparator = "|"
+)
+
 // NativeReportSummary is the default supported scan report summary model.
 // Generated based on the report with v1.MimeTypeNativeReport mime type.
 type NativeReportSummary struct {
@@ -43,7 +49,6 @@ type NativeReportSummary struct {
 func (sum *NativeReportSummary) Merge(another *NativeReportSummary) *NativeReportSummary {
 	r := &NativeReportSummary{}
 
-	r.ReportID = sum.ReportID
 	r.StartTime = minTime(sum.StartTime, another.StartTime)
 	r.EndTime = maxTime(sum.EndTime, another.EndTime)
 	r.Duration = r.EndTime.Unix() - r.StartTime.Unix()
@@ -51,32 +56,9 @@ func (sum *NativeReportSummary) Merge(another *NativeReportSummary) *NativeRepor
 	r.TotalCount = sum.TotalCount + another.TotalCount
 	r.CompleteCount = sum.CompleteCount + another.CompleteCount
 	r.CompletePercent = r.CompleteCount * 100 / r.TotalCount
-
-	if sum.Severity.String() != "" && another.Severity.String() != "" {
-		if sum.Severity.Code() > another.Severity.Code() {
-			r.Severity = sum.Severity
-		} else {
-			r.Severity = another.Severity
-		}
-	} else if sum.Severity.String() != "" {
-		r.Severity = sum.Severity
-	} else {
-		r.Severity = another.Severity
-	}
-
-	if isRunningStatus(sum.ScanStatus) || isRunningStatus(another.ScanStatus) {
-		r.ScanStatus = job.RunningStatus.String()
-	} else {
-		diff := job.Status(sum.ScanStatus).Compare(job.Status(another.ScanStatus))
-		if diff < 0 {
-			r.ScanStatus = another.ScanStatus
-		} else if diff == 0 {
-			if job.Status(sum.ScanStatus) == job.SuccessStatus ||
-				job.Status(another.ScanStatus) == job.SuccessStatus {
-				r.ScanStatus = job.SuccessStatus.String()
-			}
-		}
-	}
+	r.ReportID = mergeReportID(sum.ReportID, another.ReportID)
+	r.Severity = mergeSeverity(sum.Severity, another.Severity)
+	r.ScanStatus = mergeScanStatus(sum.ScanStatus, another.ScanStatus)
 
 	if sum.Summary != nil && another.Summary != nil {
 		r.Summary = sum.Summary.Merge(another.Summary)
@@ -139,6 +121,44 @@ func maxTime(t1, t2 time.Time) time.Time {
 	return t1
 }
 
-func isRunningStatus(status string) bool {
-	return job.Status(status) == job.RunningStatus
+func mergeReportID(r1, r2 string) string {
+	src, err := base64.StdEncoding.DecodeString(r1)
+	if err != nil {
+		src = []byte(r1)
+	}
+	src = append(src, []byte(SummaryReportIDSeparator+r2)...)
+
+	return base64.StdEncoding.EncodeToString(src)
+}
+
+func mergeSeverity(s1, s2 Severity) Severity {
+	severityValue := func(s Severity) int {
+		if s.String() == "" {
+			return -1
+		}
+
+		return s.Code()
+	}
+
+	if severityValue(s1) > severityValue(s2) {
+		return s1
+	}
+
+	return s2
+}
+
+func mergeScanStatus(s1, s2 string) string {
+	j1, j2 := job.Status(s1), job.Status(s2)
+
+	if j1 == job.RunningStatus || j2 == job.RunningStatus {
+		return job.RunningStatus.String()
+	} else if j1 == job.SuccessStatus || j2 == job.SuccessStatus {
+		return job.SuccessStatus.String()
+	}
+
+	if j1.Compare(j2) > 0 {
+		return s1
+	}
+
+	return s2
 }

src/pkg/scan/vuln/summary_test.go

@@ -0,0 +1,149 @@
+// 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 vuln
+
+import (
+	"encoding/base64"
+	"testing"
+
+	"github.com/goharbor/harbor/src/jobservice/job"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_mergeReportID(t *testing.T) {
+	type args struct {
+		r1 string
+		r2 string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{"1|2", args{"1", "2"}, base64.StdEncoding.EncodeToString([]byte("1|2"))},
+		{"1|2|3", args{base64.StdEncoding.EncodeToString([]byte("1|2")), "3"}, base64.StdEncoding.EncodeToString([]byte("1|2|3"))},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := mergeReportID(tt.args.r1, tt.args.r2); got != tt.want {
+				t.Errorf("mergeReportID() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_mergeSeverity(t *testing.T) {
+	type args struct {
+		s1 Severity
+		s2 Severity
+	}
+	tests := []struct {
+		name string
+		args args
+		want Severity
+	}{
+		{"empty string and none", args{Severity(""), None}, None},
+		{"none and empty string", args{None, Severity("")}, None},
+		{"none and low", args{None, Low}, Low},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := mergeSeverity(tt.args.s1, tt.args.s2); got != tt.want {
+				t.Errorf("mergeSeverity() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_mergeScanStatus(t *testing.T) {
+	errorStatus := job.ErrorStatus.String()
+	runningStatus := job.RunningStatus.String()
+	successStatus := job.SuccessStatus.String()
+
+	type args struct {
+		s1 string
+		s2 string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{"running and error", args{runningStatus, errorStatus}, runningStatus},
+		{"running and success", args{runningStatus, successStatus}, runningStatus},
+		{"running and running", args{runningStatus, runningStatus}, runningStatus},
+		{"success and error", args{successStatus, errorStatus}, successStatus},
+		{"success and success", args{successStatus, successStatus}, successStatus},
+		{"error and error", args{errorStatus, errorStatus}, errorStatus},
+		{"error and empty string", args{errorStatus, ""}, errorStatus},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := mergeScanStatus(tt.args.s1, tt.args.s2); got != tt.want {
+				t.Errorf("mergeScanStatus() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestMergeVulnerabilitySummary(t *testing.T) {
+	assert := assert.New(t)
+	v1 := VulnerabilitySummary{
+		Total:   1,
+		Fixable: 1,
+		Summary: map[Severity]int{Low: 1},
+	}
+
+	r := v1.Merge(&VulnerabilitySummary{
+		Total:   1,
+		Fixable: 1,
+		Summary: map[Severity]int{Low: 1, High: 1},
+	})
+
+	assert.Equal(2, r.Total)
+	assert.Equal(2, r.Fixable)
+	assert.Len(r.Summary, 2)
+	assert.Equal(2, r.Summary[Low])
+	assert.Equal(1, r.Summary[High])
+}
+
+func TestMergeNativeReportSummary(t *testing.T) {
+	assert := assert.New(t)
+	errorStatus := job.ErrorStatus.String()
+	runningStatus := job.RunningStatus.String()
+
+	v1 := VulnerabilitySummary{
+		Total:   1,
+		Fixable: 1,
+		Summary: map[Severity]int{Low: 1},
+	}
+
+	n1 := NativeReportSummary{
+		ScanStatus: runningStatus,
+		Severity:   Low,
+		TotalCount: 1,
+		Summary:    &v1,
+	}
+
+	r := n1.Merge(&NativeReportSummary{
+		ScanStatus: errorStatus,
+		Severity:   Severity(""),
+		TotalCount: 1,
+	})
+
+	assert.Equal(runningStatus, r.ScanStatus)
+	assert.Equal(Low, r.Severity)
+	assert.Equal(v1, *r.Summary)
+}

src/portal/src/i18n/lang/en-us-lang.json

@@ -992,7 +992,7 @@
         "CHART": {
             "SCANNING_TIME": "Scan completed time:",
             "SCANNING_PERCENT": "Scan completed percent:",
-            "SCANNING_PERCENT_EXPLAIN": "(Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index not including the index file itself and the images that do not support being scanned.)",
+            "SCANNING_PERCENT_EXPLAIN": "Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index.",
             "TOOLTIPS_TITLE": "{{totalVulnerability}} of {{totalPackages}} {{package}} have known {{vulnerability}}.",
             "TOOLTIPS_TITLE_SINGULAR": "{{totalVulnerability}} of {{totalPackages}} {{package}} has known {{vulnerability}}.",
             "TOOLTIPS_TITLE_ZERO": "No recognizable vulnerability found"

src/portal/src/i18n/lang/es-es-lang.json

@@ -991,7 +991,7 @@
         "CHART": {
             "SCANNING_TIME": "Scan completed time:",
             "SCANNING_PERCENT": "Scan completed percent:",
-            "SCANNING_PERCENT_EXPLAIN": "(Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index not including the index file itself and the images that do not support being scanned.)",
+            "SCANNING_PERCENT_EXPLAIN": "Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index.",
             "TOOLTIPS_TITLE": "{{totalVulnerability}} of {{totalPackages}} {{package}} have known {{vulnerability}}.",
             "TOOLTIPS_TITLE_SINGULAR": "{{totalVulnerability}} of {{totalPackages}} {{package}} has known {{vulnerability}}.",
             "TOOLTIPS_TITLE_ZERO": "No recognizable vulnerability found"

src/portal/src/i18n/lang/fr-fr-lang.json

@@ -964,7 +964,7 @@
         "CHART": {
             "SCANNING_TIME": "Temps d'analyse complète :",
             "SCANNING_PERCENT": "Scan completed percent:",
-            "SCANNING_PERCENT_EXPLAIN": "(Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index not including the index file itself and the images that do not support being scanned.)",
+            "SCANNING_PERCENT_EXPLAIN": "Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index.",
             "TOOLTIPS_TITLE": "{{totalVulnerability}} de {{totalPackages}} {{package}} ont des {{vulnerability}} connues.",
             "TOOLTIPS_TITLE_SINGULAR": "{{totalVulnerability}} de {{totalPackages}} {{package}} a des {{vulnerability}} connues.",
             "TOOLTIPS_TITLE_ZERO": "No recognizable vulnerability found"

src/portal/src/i18n/lang/pt-br-lang.json

@@ -987,7 +987,7 @@
         "CHART": {
             "SCANNING_TIME": "Tempo de conclusão da análise:",
             "SCANNING_PERCENT": "Scan completed percent:",
-            "SCANNING_PERCENT_EXPLAIN": "(Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index not including the index file itself and the images that do not support being scanned.)",
+            "SCANNING_PERCENT_EXPLAIN": "Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index.",
             "TOOLTIPS_TITLE": "{{totalVulnerability}} de {{totalPackages}} {{package}} possuem {{vulnerability}}.",
             "TOOLTIPS_TITLE_SINGULAR": "{{totalVulnerability}} de {{totalPackages}} {{package}} possuem {{vulnerability}}.",
             "TOOLTIPS_TITLE_ZERO": "No recognizable vulnerability found"

src/portal/src/i18n/lang/tr-tr-lang.json

@@ -992,7 +992,7 @@
         "CHART": {
             "SCANNING_TIME": "Tarama tamamlanma zamanı:",
             "SCANNING_PERCENT": "Scan completed percent:",
-            "SCANNING_PERCENT_EXPLAIN": "(Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index not including the index file itself and the images that do not support being scanned.)",
+            "SCANNING_PERCENT_EXPLAIN": "Scan completed percentage is calculated as # of successfully scanned images / total number of images referenced within the image index.",
             "TOOLTIPS_TITLE": "{{totalVulnerability}} 'nın {{totalPackages}} {{package}} bilinen {{vulnerability}}.",
             "TOOLTIPS_TITLE_SINGULAR": "{{totalVulnerability}} 'nın {{totalPackages}} {{package}} bilinen {{vulnerability}}.",
             "TOOLTIPS_TITLE_ZERO": "No recognizable vulnerability found"

src/portal/src/i18n/lang/zh-cn-lang.json

@@ -991,7 +991,7 @@
         "CHART": {
             "SCANNING_TIME": "扫描完成时间:",
             "SCANNING_PERCENT": "扫描完成度:",
-            "SCANNING_PERCENT_EXPLAIN": "（扫描完成度是扫描成功的镜像数与总共需要扫描的镜像数的比值。总共需要的扫描的镜像不包含 Index 本身和不支持扫描的镜像。）",
+            "SCANNING_PERCENT_EXPLAIN": "扫描完成度是扫描成功的镜像数与总共需要扫描的镜像数的比值，总共需要的扫描的镜像不包含 Index 。",
             "TOOLTIPS_TITLE": "{{totalPackages}}个{{package}}中的{{totalVulnerability}}个含有{{vulnerability}}。",
             "TOOLTIPS_TITLE_SINGULAR": "{{totalPackages}}个{{package}}中的{{totalVulnerability}}个含有{{vulnerability}}。",
             "TOOLTIPS_TITLE_ZERO": "没有发现可识别的漏洞"