diff --git a/src/controller/scan/base_controller.go b/src/controller/scan/base_controller.go index 0d57344c3..45255c09b 100644 --- a/src/controller/scan/base_controller.go +++ b/src/controller/scan/base_controller.go @@ -139,7 +139,7 @@ func NewController() Controller { execMgr: task.ExecMgr, taskMgr: task.Mgr, // Get the scan V1 to V2 report converters - reportConverter: postprocessors.NewNativeToRelationalSchemaConverter(), + reportConverter: postprocessors.Converter, } } @@ -655,44 +655,6 @@ func (bc *basicController) GetScanLog(ctx context.Context, uuid string) ([]byte, return b.Bytes(), nil } -func (bc *basicController) UpdateReport(ctx context.Context, report *sca.CheckInReport) error { - rpl, err := bc.manager.GetBy(ctx, report.Digest, report.RegistrationUUID, []string{report.MimeType}) - if err != nil { - return errors.Wrap(err, "scan controller: handle job hook") - } - - logger := log.G(ctx) - - if len(rpl) == 0 { - fields := log.Fields{ - "report_digest": report.Digest, - "registration_uuid": report.RegistrationUUID, - "mime_type": report.MimeType, - } - logger.WithFields(fields).Warningf("no report found to update data") - - return errors.NotFoundError(nil).WithMessage("no report found to update data") - } - - logger.Debugf("Converting report ID %s to the new V2 schema", rpl[0].UUID) - - _, reportData, err := bc.reportConverter.ToRelationalSchema(ctx, rpl[0].UUID, rpl[0].RegistrationUUID, rpl[0].Digest, report.RawReport) - if err != nil { - return errors.Wrapf(err, "Failed to convert vulnerability data to new schema for report UUID : %s", rpl[0].UUID) - } - // update the original report with the new summarized report with all vulnerability data removed. - // this is required since the top level layers relay on the vuln.Report struct that - // contains additional metadata within the report which if stored in the new columns within the scan_report table - // would be redundant - if err := bc.manager.UpdateReportData(ctx, rpl[0].UUID, reportData); err != nil { - return errors.Wrap(err, "scan controller: handle job hook") - } - - logger.Debugf("Converted report ID %s to the new V2 schema", rpl[0].UUID) - - return nil -} - // DeleteReports ... func (bc *basicController) DeleteReports(ctx context.Context, digests ...string) error { if err := bc.manager.DeleteByDigests(ctx, digests...); err != nil { @@ -747,6 +709,10 @@ func (bc *basicController) GetVulnerable(ctx context.Context, artifact *ar.Artif return nil, err } + if raw == nil { + return vulnerable, nil + } + rp, ok := raw.(*vuln.Report) if !ok { return nil, errors.Errorf("type mismatch: expect *vuln.Report but got %s", reflect.TypeOf(raw).String()) @@ -1006,6 +972,7 @@ func (bc *basicController) assembleReports(ctx context.Context, reports ...*scan } else { report.Status = job.ErrorStatus.String() } + completeReport, err := bc.reportConverter.FromRelationalSchema(ctx, report.UUID, report.Digest, report.Report) if err != nil { return err diff --git a/src/controller/scan/base_controller_test.go b/src/controller/scan/base_controller_test.go index e2a7f9afc..ce77059c0 100644 --- a/src/controller/scan/base_controller_test.go +++ b/src/controller/scan/base_controller_test.go @@ -19,6 +19,9 @@ import ( "encoding/base64" "encoding/json" "fmt" + "testing" + "time" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/controller/artifact" @@ -47,8 +50,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "testing" - "time" ) // ControllerTestSuite is the test suite for scan controller. @@ -456,22 +457,6 @@ func (suite *ControllerTestSuite) TestScanControllerGetMultiScanLog() { } } -func (suite *ControllerTestSuite) TestUpdateReport() { - { - // get report failed - suite.reportMgr.On("GetBy", context.TODO(), "digest", "ruuid", []string{"mime"}).Return(nil, fmt.Errorf("failed")).Once() - report := &sca.CheckInReport{Digest: "digest", RegistrationUUID: "ruuid", MimeType: "mime"} - suite.Error(suite.c.UpdateReport(context.TODO(), report)) - } - - { - // report not found - suite.reportMgr.On("GetBy", context.TODO(), "digest", "ruuid", []string{"mime"}).Return(nil, nil).Once() - report := &sca.CheckInReport{Digest: "digest", RegistrationUUID: "ruuid", MimeType: "mime"} - suite.Error(suite.c.UpdateReport(context.TODO(), report)) - } -} - func (suite *ControllerTestSuite) TestScanAll() { { // no artifacts found when scan all diff --git a/src/controller/scan/callback.go b/src/controller/scan/callback.go index de292f547..631a338e7 100644 --- a/src/controller/scan/callback.go +++ b/src/controller/scan/callback.go @@ -23,7 +23,6 @@ import ( "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/notification" - "github.com/goharbor/harbor/src/pkg/scan" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" "github.com/goharbor/harbor/src/pkg/scheduler" "github.com/goharbor/harbor/src/pkg/task" @@ -47,18 +46,10 @@ func init() { } // NOTE: the vendor type of execution for the scan job trigger by the scan all is VendorTypeScanAll - if err := task.RegisterCheckInProcessor(VendorTypeScanAll, scanTaskCheckInProcessor); err != nil { - log.Fatalf("failed to register the checkin processor for the scan all job, error %v", err) - } - if err := task.RegisterTaskStatusChangePostFunc(VendorTypeScanAll, scanTaskStatusChange); err != nil { log.Fatalf("failed to register the task status change post for the scan all job, error %v", err) } - if err := task.RegisterCheckInProcessor(job.ImageScanJob, scanTaskCheckInProcessor); err != nil { - log.Fatalf("failed to register the checkin processor for the scan job, error %v", err) - } - if err := task.RegisterTaskStatusChangePostFunc(job.ImageScanJob, scanTaskStatusChange); err != nil { log.Fatalf("failed to register the task status change post for the scan job, error %v", err) } @@ -114,14 +105,3 @@ func scanTaskStatusChange(ctx context.Context, taskID int64, status string) (err return nil } - -// scanTaskCheckInProcessor checkin processor handles the webhook of scan job -func scanTaskCheckInProcessor(ctx context.Context, t *task.Task, sc *job.StatusChange) (err error) { - checkInReport := &scan.CheckInReport{} - if err := checkInReport.FromJSON(sc.CheckIn); err != nil { - log.G(ctx).WithField("error", err).Errorf("failed to convert data to report") - return err - } - - return scanCtl.UpdateReport(ctx, checkInReport) -} diff --git a/src/controller/scan/callback_test.go b/src/controller/scan/callback_test.go index 4d2d55fc2..f9c2c7012 100644 --- a/src/controller/scan/callback_test.go +++ b/src/controller/scan/callback_test.go @@ -22,8 +22,6 @@ import ( "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/jobservice/job" - "github.com/goharbor/harbor/src/pkg/scan" - dscan "github.com/goharbor/harbor/src/pkg/scan/dao/scan" "github.com/goharbor/harbor/src/pkg/task" artifacttesting "github.com/goharbor/harbor/src/testing/controller/artifact" robottesting "github.com/goharbor/harbor/src/testing/controller/robot" @@ -133,33 +131,6 @@ func (suite *CallbackTestSuite) TestScanTaskStatusChange() { } } -func (suite *CallbackTestSuite) TestScanTaskCheckInProcessor() { - { - suite.Error(scanTaskCheckInProcessor(context.TODO(), &task.Task{}, &job.StatusChange{CheckIn: "report"})) - } - - { - suite.reportMgr.On("GetBy", context.TODO(), "digest", "ruuid", []string{"mime_type"}).Return( - []*dscan.Report{ - {UUID: "uuid"}, - }, - nil, - ).Once() - - suite.reportMgr.On("UpdateReportData", context.TODO(), "uuid", "raw_report").Return(nil) - - report := scan.CheckInReport{ - Digest: "digest", - RegistrationUUID: "ruuid", - MimeType: "mime_type", - RawReport: "raw_report", - } - - r, _ := json.Marshal(report) - suite.NoError(scanTaskCheckInProcessor(context.TODO(), &task.Task{}, &job.StatusChange{CheckIn: string(r)})) - } -} - func (suite *CallbackTestSuite) TestScanAllCallback() { { // create execution failed diff --git a/src/controller/scan/controller.go b/src/controller/scan/controller.go index e0e3fbe05..191047d04 100644 --- a/src/controller/scan/controller.go +++ b/src/controller/scan/controller.go @@ -20,7 +20,6 @@ import ( "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/jobservice/job" allowlist "github.com/goharbor/harbor/src/pkg/allowlist/models" - sca "github.com/goharbor/harbor/src/pkg/scan" "github.com/goharbor/harbor/src/pkg/scan/dao/scan" "github.com/goharbor/harbor/src/pkg/scan/vuln" ) @@ -95,16 +94,6 @@ type Controller interface { // error : non nil error if any errors occurred DeleteReports(ctx context.Context, digests ...string) error - // UpdateReport update the report - // - // Arguments: - // ctx context.Context : the context for this method - // report *sca.CheckInReport : the scan report - // - // Returns: - // error : non nil error if any errors occurred - UpdateReport(ctx context.Context, report *sca.CheckInReport) error - // Scan all the artifacts // // Arguments: diff --git a/src/lib/set.go b/src/lib/set.go new file mode 100644 index 000000000..fb1b4b923 --- /dev/null +++ b/src/lib/set.go @@ -0,0 +1,42 @@ +// 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 lib + +type void = struct{} + +// Set a simple set +type Set map[interface{}]void + +// Add add item to set +func (s Set) Add(item interface{}) { + s[item] = void{} +} + +// Exists returns true when item in the set +func (s Set) Exists(item interface{}) bool { + _, ok := s[item] + + return ok +} + +// Items returns the items in the set +func (s Set) Items() []interface{} { + var items []interface{} + for item := range s { + items = append(items, item) + } + + return items +} diff --git a/src/lib/set_test.go b/src/lib/set_test.go new file mode 100644 index 000000000..edfebaf39 --- /dev/null +++ b/src/lib/set_test.go @@ -0,0 +1,33 @@ +// 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 lib + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSet(t *testing.T) { + assert := assert.New(t) + + s := Set{} + + s.Add(1) + + assert.True(s.Exists(1)) + + assert.Len(s.Items(), 1) +} diff --git a/src/pkg/scan/dao/scan/model.go b/src/pkg/scan/dao/scan/model.go index 1c92938fa..e392cb78e 100644 --- a/src/pkg/scan/dao/scan/model.go +++ b/src/pkg/scan/dao/scan/model.go @@ -14,7 +14,10 @@ package scan -import "time" +import ( + "fmt" + "time" +) // Report of the scan. // Identified by the `digest`, `registration_uuid` and `mime_type`. @@ -87,6 +90,11 @@ func (vr *VulnerabilityRecord) GetID() int64 { return vr.ID } +// Key returns the uniq key of the vuln +func (vr *VulnerabilityRecord) Key() string { + return fmt.Sprintf("%s-%s-%s", vr.CVEID, vr.Package, vr.PackageVersion) +} + // ReportVulnerabilityRecord is relation table required to optimize data storage for both the // vulnerability records and the scan report. // identified by composite key (ID, Report) diff --git a/src/pkg/scan/dao/scan/vulnerability.go b/src/pkg/scan/dao/scan/vulnerability.go index 264633ed5..41f027729 100644 --- a/src/pkg/scan/dao/scan/vulnerability.go +++ b/src/pkg/scan/dao/scan/vulnerability.go @@ -16,8 +16,9 @@ package scan import ( "context" - "fmt" + "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" ) @@ -33,10 +34,12 @@ type VulnerabilityRecordDao interface { Create(ctx context.Context, vr *VulnerabilityRecord) (int64, error) // Delete deletes a vulnerability record Delete(ctx context.Context, vr *VulnerabilityRecord) error + // Update updates a vulnerability record + Update(ctx context.Context, vr *VulnerabilityRecord, cols ...string) error // List lists the vulnerability records List(ctx context.Context, query *q.Query) ([]*VulnerabilityRecord, error) // InsertForReport inserts vulnerability records for a report - InsertForReport(ctx context.Context, reportUUID string, vr *VulnerabilityRecord) (int64, error) + InsertForReport(ctx context.Context, reportUUID string, vulnerabilityRecordIDs ...int64) error // GetForReport gets vulnerability records for a report GetForReport(ctx context.Context, reportUUID string) ([]*VulnerabilityRecord, error) // GetForScanner gets vulnerability records for a scanner @@ -76,6 +79,16 @@ func (v *vulnerabilityRecordDao) Delete(ctx context.Context, vr *VulnerabilityRe return err } +func (v *vulnerabilityRecordDao) Update(ctx context.Context, vr *VulnerabilityRecord, cols ...string) error { + o, err := orm.FromContext(ctx) + if err != nil { + return err + } + _, err = o.Update(vr, cols...) + + return err +} + // List lists the vulnerability records with given query parameters. // Keywords in query here will be enforced with `exact` way. // If the registration ID (which = the scanner ID is not specified), the results @@ -84,52 +97,60 @@ func (v *vulnerabilityRecordDao) Delete(ctx context.Context, vr *VulnerabilityRe // responsibility of the calling code to de-duplicate the CVE records or bucket them // per registered scanner func (v *vulnerabilityRecordDao) List(ctx context.Context, query *q.Query) ([]*VulnerabilityRecord, error) { - o, err := orm.FromContext(ctx) + qs, err := orm.QuerySetter(ctx, &VulnerabilityRecord{}, query) if err != nil { return nil, err } - qt := o.QueryTable(new(VulnerabilityRecord)) - - if query != nil { - if len(query.Keywords) > 0 { - 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) - } - } - - if query.PageNumber > 0 && query.PageSize > 0 { - qt = qt.Limit(query.PageSize, (query.PageNumber-1)*query.PageSize) - } - } l := make([]*VulnerabilityRecord, 0) - _, err = qt.All(&l) + _, err = qs.All(&l) return l, err } -// InsertForReport inserts a vulnerability record in the context of scan report -func (v *vulnerabilityRecordDao) InsertForReport(ctx context.Context, reportUUID string, vr *VulnerabilityRecord) (int64, error) { - - vrID, err := v.Create(ctx, vr) - - if err != nil { - return 0, err +// InsertForReport inserts the vulnerability records in the context of scan report +func (v *vulnerabilityRecordDao) InsertForReport(ctx context.Context, reportUUID string, vulnerabilityRecordIDs ...int64) error { + if len(vulnerabilityRecordIDs) == 0 { + return nil } - rvr := new(ReportVulnerabilityRecord) - rvr.Report = reportUUID - rvr.VulnRecordID = vrID + s := lib.Set{} - _, rvrID, err := orm.ReadOrCreate(ctx, rvr, "report_uuid", "vuln_record_id") + var records []*ReportVulnerabilityRecord + for _, vulnerabilityRecordID := range vulnerabilityRecordIDs { + if s.Exists(vulnerabilityRecordID) { + continue + } - return rvrID, err + s.Add(vulnerabilityRecordID) + records = append(records, &ReportVulnerabilityRecord{ + Report: reportUUID, + VulnRecordID: vulnerabilityRecordID, + }) + } + + h := func(ctx context.Context) error { + o, err := orm.FromContext(ctx) + if err != nil { + return err + } + + _, err = o.InsertMulti(100, records) + return err + } + + if err := orm.WithTransaction(h)(ctx); err != nil { + fields := log.Fields{ + "error": err, + "report": reportUUID, + } + log.G(ctx).WithFields(fields).Warningf("Could not associate vulnerability record to the report") + + return err + } + + return nil } // DeleteForReport deletes the vulnerability records for a single report diff --git a/src/pkg/scan/dao/scan/vulnerability_test.go b/src/pkg/scan/dao/scan/vulnerability_test.go index 158307a50..1fda2af49 100644 --- a/src/pkg/scan/dao/scan/vulnerability_test.go +++ b/src/pkg/scan/dao/scan/vulnerability_test.go @@ -19,13 +19,10 @@ import ( "testing" "github.com/goharbor/harbor/src/jobservice/job" - "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/scan/dao/scanner" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" htesting "github.com/goharbor/harbor/src/testing" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -125,13 +122,13 @@ func (suite *VulnerabilityTestSuite) SetupTest() { // TearDownTest clears enf for test case. func (suite *VulnerabilityTestSuite) TearDownTest() { registrations, err := scanner.ListRegistrations(suite.Context(), &q.Query{}) - require.NoError(suite.T(), err, "Failed to cleanup scanner registrations") + suite.NoError(err, "Failed to cleanup scanner registrations") for _, registration := range registrations { err = scanner.DeleteRegistration(suite.Context(), registration.UUID) - require.NoError(suite.T(), err, "Error when cleaning up scanner registrations") + suite.NoError(err, "Error when cleaning up scanner registrations") } - reports, err := suite.dao.List(orm.Context(), &q.Query{}) - require.NoError(suite.T(), err) + reports, err := suite.dao.List(suite.Context(), &q.Query{}) + suite.NoError(err) for _, report := range reports { suite.cleanUpAdditionalData(report.UUID, report.RegistrationUUID) } @@ -160,14 +157,14 @@ func (suite *VulnerabilityTestSuite) TestVulnerabilityRecordsListForReport() { // fetch the records for the first report. Additionally assert that these records // indeed belong to the same report being fetched and not to another report { - vulns, err := suite.vulnerabilityRecordDao.GetForReport(orm.Context(), "uuid") - require.NoError(suite.T(), err, "Error when fetching vulnerability records for report") - require.True(suite.T(), len(vulns) > 0) + vulns, err := suite.vulnerabilityRecordDao.GetForReport(suite.Context(), "uuid") + suite.NoError(err, "Error when fetching vulnerability records for report") + suite.True(len(vulns) > 0) } { - vulns, err := suite.vulnerabilityRecordDao.GetForReport(orm.Context(), "uuid1") - require.NoError(suite.T(), err, "Error when fetching vulnerability records for report") - require.True(suite.T(), len(vulns) > 0) + vulns, err := suite.vulnerabilityRecordDao.GetForReport(suite.Context(), "uuid1") + suite.NoError(err, "Error when fetching vulnerability records for report") + suite.True(len(vulns) > 0) } } @@ -175,16 +172,16 @@ func (suite *VulnerabilityTestSuite) TestVulnerabilityRecordsListForReport() { // TestGetVulnerabilityRecordsForScanner gets vulnerability records for scanner func (suite *VulnerabilityTestSuite) TestGetVulnerabilityRecordsForScanner() { - vulns, err := suite.vulnerabilityRecordDao.GetForScanner(orm.Context(), "scannerId1") - require.NoError(suite.T(), err, "Error when fetching vulnerability records for report") - require.True(suite.T(), len(vulns) > 0) + vulns, err := suite.vulnerabilityRecordDao.GetForScanner(suite.Context(), "scannerId1") + suite.NoError(err, "Error when fetching vulnerability records for report") + suite.True(len(vulns) > 0) } // TestGetVulnerabilityRecordIdsForScanner gets vulnerability records for scanner func (suite *VulnerabilityTestSuite) TestGetVulnerabilityRecordIdsForScanner() { - vulns, err := suite.vulnerabilityRecordDao.GetRecordIdsForScanner(orm.Context(), "scannerId1") - require.NoError(suite.T(), err, "Error when fetching vulnerability records for report") - require.True(suite.T(), len(vulns) > 0) + vulns, err := suite.vulnerabilityRecordDao.GetRecordIdsForScanner(suite.Context(), "scannerId1") + suite.NoError(err, "Error when fetching vulnerability records for report") + suite.True(len(vulns) > 0) } // TestDeleteForDigest tests deleting vulnerability report for a specific digest @@ -206,9 +203,9 @@ func (suite *VulnerabilityTestSuite) TestDeleteForDigest() { for _, v := range vulns { suite.insertVulnRecordForReport("uuid1", v) } - delCount, err := suite.vulnerabilityRecordDao.DeleteForDigests(orm.Context(), "digest1") - require.NoError(suite.T(), err) - assert.Equal(suite.T(), int64(10), delCount) + delCount, err := suite.vulnerabilityRecordDao.DeleteForDigests(suite.Context(), "digest1") + suite.NoError(err) + suite.Equal(int64(10), delCount) } func (suite *VulnerabilityTestSuite) TestDuplicateRecords() { @@ -230,46 +227,48 @@ func (suite *VulnerabilityTestSuite) TestDuplicateRecords() { // TestDeleteVulnerabilityRecord gets vulnerability records for scanner func (suite *VulnerabilityTestSuite) TestDeleteVulnerabilityRecord() { - vulns, err := suite.vulnerabilityRecordDao.GetForScanner(orm.Context(), "scannerId1") - require.NoError(suite.T(), err, "Error when fetching vulnerability records for report") - require.True(suite.T(), len(vulns) > 0) + vulns, err := suite.vulnerabilityRecordDao.GetForScanner(suite.Context(), "scannerId1") + suite.NoError(err, "Error when fetching vulnerability records for report") + suite.True(len(vulns) > 0) for _, vuln := range vulns { - err = suite.vulnerabilityRecordDao.Delete(orm.Context(), vuln) - require.NoError(suite.T(), err) + err = suite.vulnerabilityRecordDao.Delete(suite.Context(), vuln) + suite.NoError(err) } } // TestListVulnerabilityRecord gets vulnerability records for scanner func (suite *VulnerabilityTestSuite) TestListVulnerabilityRecord() { - vulns, err := suite.vulnerabilityRecordDao.List(orm.Context(), &q.Query{Keywords: map[string]interface{}{"CVEID": "CVE-ID1"}}) - require.NoError(suite.T(), err, "Error when fetching vulnerability records for report") - require.True(suite.T(), len(vulns) > 0) + vulns, err := suite.vulnerabilityRecordDao.List(suite.Context(), &q.Query{Keywords: map[string]interface{}{"CVEID": "CVE-ID1"}}) + suite.NoError(err, "Error when fetching vulnerability records for report") + suite.True(len(vulns) > 0) } func (suite *VulnerabilityTestSuite) createReport(r *Report) { - id, err := suite.dao.Create(orm.Context(), r) - require.NoError(suite.T(), err) - require.Condition(suite.T(), func() (success bool) { + id, err := suite.dao.Create(suite.Context(), r) + suite.NoError(err) + suite.Condition(func() (success bool) { success = id > 0 return }) } func (suite *VulnerabilityTestSuite) insertVulnRecordForReport(reportUUID string, vr *VulnerabilityRecord) { - id, err := suite.vulnerabilityRecordDao.InsertForReport(orm.Context(), reportUUID, vr) - require.NoError(suite.T(), err) - require.True(suite.T(), id > 0, "Failed to insert vulnerability record row for report %s", reportUUID) + id, err := suite.vulnerabilityRecordDao.Create(suite.Context(), vr) + suite.NoError(err, "Failed to create vulnerability record") + + err = suite.vulnerabilityRecordDao.InsertForReport(suite.Context(), reportUUID, id) + suite.NoError(err, "Failed to insert vulnerability record row for report %s", reportUUID) } func (suite *VulnerabilityTestSuite) cleanUpAdditionalData(reportID string, scannerID string) { - _, err := suite.dao.DeleteMany(orm.Context(), q.Query{Keywords: q.KeyWords{"uuid": reportID}}) + _, err := suite.dao.DeleteMany(suite.Context(), q.Query{Keywords: q.KeyWords{"uuid": reportID}}) - require.NoError(suite.T(), err) - _, err = suite.vulnerabilityRecordDao.DeleteForReport(orm.Context(), reportID) - require.NoError(suite.T(), err, "Failed to cleanup records") - _, err = suite.vulnerabilityRecordDao.DeleteForScanner(orm.Context(), scannerID) - require.NoError(suite.T(), err, "Failed to delete vulnerability records") + suite.NoError(err) + _, err = suite.vulnerabilityRecordDao.DeleteForReport(suite.Context(), reportID) + suite.NoError(err, "Failed to cleanup records") + _, err = suite.vulnerabilityRecordDao.DeleteForScanner(suite.Context(), scannerID) + suite.NoError(err, "Failed to delete vulnerability records") } func (suite *VulnerabilityTestSuite) registerScanner(registrationUUID string) { @@ -281,7 +280,7 @@ func (suite *VulnerabilityTestSuite) registerScanner(registrationUUID string) { } _, err := scanner.AddRegistration(suite.Context(), r) - require.NoError(suite.T(), err, "add new registration") + suite.NoError(err, "add new registration") } func generateVulnerabilityRecordsForReport(reportUUID string, registrationUUID string, numRecords int) []*VulnerabilityRecord { diff --git a/src/pkg/scan/job.go b/src/pkg/scan/job.go index 90d116738..1d2a116a4 100644 --- a/src/pkg/scan/job.go +++ b/src/pkg/scan/job.go @@ -35,6 +35,7 @@ import ( "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/pkg/robot/model" "github.com/goharbor/harbor/src/pkg/scan/dao/scanner" + "github.com/goharbor/harbor/src/pkg/scan/postprocessors" "github.com/goharbor/harbor/src/pkg/scan/report" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" ) @@ -148,12 +149,12 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error { // Ignore errors as they have been validated already r, _ := extractRegistration(params) req, _ := ExtractScanReq(params) - mimes, _ := extractMimeTypes(params) + mimeTypes, _ := extractMimeTypes(params) // Print related infos to log printJSONParameter(JobParamRegistration, params[JobParamRegistration].(string), myLogger) printJSONParameter(JobParameterRequest, removeAuthInfo(req), myLogger) - myLogger.Infof("Report mime types: %v\n", mimes) + myLogger.Infof("Report mime types: %v\n", mimeTypes) // Submit scan request to the scanner adapter client, err := r.Client(v1.DefaultClientPool) @@ -188,13 +189,14 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error { } // For collecting errors - errs := make([]error, len(mimes)) + errs := make([]error, len(mimeTypes)) + rawReports := make([]string, len(mimeTypes)) // Concurrently retrieving report by different mime types wg := &sync.WaitGroup{} - wg.Add(len(mimes)) + wg.Add(len(mimeTypes)) - for i, mt := range mimes { + for i, mimeType := range mimeTypes { go func(i int, m string) { defer wg.Done() @@ -231,28 +233,8 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error { return } - // Check in - cir := &CheckInReport{ - Digest: req.Artifact.Digest, - RegistrationUUID: r.UUID, - MimeType: m, - RawReport: rawReport, - } + rawReports[i] = rawReport - var ( - jsonData string - er error - ) - if jsonData, er = cir.ToJSON(); er == nil { - if er = ctx.Checkin(jsonData); er == nil { - // Done! - myLogger.Infof("Report with mime type %s is checked in", m) - return - } - } - - // Send error and exit - errs[i] = errors.Wrap(er, fmt.Sprintf("check in scan report for mime type %s", m)) return case <-ctx.SystemContext().Done(): // Terminated by system @@ -262,7 +244,7 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error { return } } - }(i, mt) + }(i, mimeType) } // Wait for all the retrieving routines are completed @@ -282,9 +264,50 @@ func (j *Job) Run(ctx job.Context, params job.Parameters) error { // Log error to the job log if err != nil { myLogger.Error(err) + + return err } - return err + for i, mimeType := range mimeTypes { + reports, err := report.Mgr.GetBy(ctx.SystemContext(), req.Artifact.Digest, r.UUID, []string{mimeType}) + if err != nil { + myLogger.Error("Failed to get report for artifact %s of mimetype %s, error %v", req.Artifact.Digest, mimeType, err) + + return err + } + + if len(reports) == 0 { + myLogger.Error("No report found for artifact %s of mimetype %s, error %v", req.Artifact.Digest, mimeType, err) + + return errors.NotFoundError(nil).WithMessage("no report found to update data") + } + + rp := reports[0] + + logger.Debugf("Converting report ID %s to the new V2 schema", rp.UUID) + + // use a new ormer here to use the short db connection + _, reportData, err := postprocessors.Converter.ToRelationalSchema(ctx.SystemContext(), rp.UUID, rp.RegistrationUUID, rp.Digest, rawReports[i]) + if err != nil { + myLogger.Errorf("Failed to convert vulnerability data to new schema for report %s, error %v", rp.UUID, err) + + return err + } + + // update the original report with the new summarized report with all vulnerability data removed. + // this is required since the top level layers relay on the vuln.Report struct that + // contains additional metadata within the report which if stored in the new columns within the scan_report table + // would be redundant + if err := report.Mgr.UpdateReportData(ctx.SystemContext(), rp.UUID, reportData); err != nil { + myLogger.Errorf("Failed to update report data for report %s, error %v", rp.UUID, err) + + return err + } + + myLogger.Debugf("Converted report ID %s to the new V2 schema", rp.UUID) + } + + return nil } // ExtractScanReq extracts the scan request from the job parameters. diff --git a/src/pkg/scan/job_test.go b/src/pkg/scan/job_test.go index 063f4eac1..8a8bb2836 100644 --- a/src/pkg/scan/job_test.go +++ b/src/pkg/scan/job_test.go @@ -16,12 +16,12 @@ package scan import ( "encoding/json" - "github.com/goharbor/harbor/src/controller/robot" - "github.com/goharbor/harbor/src/pkg/robot/model" "testing" "time" + "github.com/goharbor/harbor/src/controller/robot" "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/pkg/robot/model" "github.com/goharbor/harbor/src/pkg/scan/dao/scanner" v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1" "github.com/goharbor/harbor/src/pkg/scan/vuln" @@ -145,17 +145,6 @@ func (suite *JobTestSuite) TestJob() { mc.On("GetScanReport", "scan_id", v1.MimeTypeNativeReport, v1.MimeTypeGenericVulnerabilityReport).Return(string(jRep), nil) mocktesting.OnAnything(suite.mcp, "Get").Return(mc, nil) - crp := &CheckInReport{ - Digest: sr.Artifact.Digest, - RegistrationUUID: r.UUID, - MimeType: v1.MimeTypeNativeReport, - RawReport: string(jRep), - } - - jsonData, err := crp.ToJSON() - require.NoError(suite.T(), err) - - ctx.On("Checkin", string(jsonData)).Return(nil) j := &Job{} err = j.Run(ctx, jp) require.NoError(suite.T(), err) diff --git a/src/pkg/scan/postprocessors/report_converters.go b/src/pkg/scan/postprocessors/report_converters.go index 2f0c6c75c..b108af5f1 100644 --- a/src/pkg/scan/postprocessors/report_converters.go +++ b/src/pkg/scan/postprocessors/report_converters.go @@ -21,12 +21,19 @@ import ( "strings" "github.com/goharbor/harbor/src/jobservice/job" + "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/lib/q" "github.com/goharbor/harbor/src/pkg/scan/dao/scan" "github.com/goharbor/harbor/src/pkg/scan/vuln" ) +var ( + // Converter is the global native scan report converter + Converter = NewNativeToRelationalSchemaConverter() +) + // NativeScanReportConverter is an interface that establishes the contract for the conversion process of a harbor native vulnerability report // It is the responsibility of the implementation to store the report in a manner easily retrievable using the // report UUID @@ -50,7 +57,7 @@ func NewNativeToRelationalSchemaConverter() NativeScanReportConverter { func (c *nativeToRelationalSchemaConverter) ToRelationalSchema(ctx context.Context, reportUUID string, registrationUUID string, digest string, reportData string) (string, string, error) { if len(reportData) == 0 { - log.Infof("There is no vulnerability report to toSchema for report UUID : %s", reportUUID) + log.G(ctx).Infof("There is no vulnerability report to toSchema for report UUID : %s", reportUUID) return reportUUID, "", nil } @@ -63,13 +70,14 @@ func (c *nativeToRelationalSchemaConverter) ToRelationalSchema(ctx context.Conte if err := c.toSchema(ctx, reportUUID, registrationUUID, digest, reportData); err != nil { return "", "", errors.Wrap(err, fmt.Sprintf("Error when converting vulnerability report")) } + rawReport.Vulnerabilities = nil data, err := json.Marshal(rawReport) if err != nil { return "", "", errors.Wrap(err, fmt.Sprintf("Error when persisting raw report summary")) } - return reportUUID, string(data), nil + return reportUUID, string(data), nil } // FromRelationalSchema converts the generic vulnerability record stored in relational form to the @@ -93,45 +101,96 @@ func (c *nativeToRelationalSchemaConverter) toSchema(ctx context.Context, report if err != nil { return err } + + var cveIDs []interface{} for _, v := range vulnReport.Vulnerabilities { - vulnV2 := new(scan.VulnerabilityRecord) - vulnV2.CVEID = v.ID - vulnV2.Description = v.Description - vulnV2.Package = v.Package - vulnV2.PackageVersion = v.Version - vulnV2.PackageType = "Unknown" - vulnV2.Fix = v.FixVersion - vulnV2.URLs = strings.Join(v.Links, "|") - vulnV2.RegistrationUUID = registrationUUID - vulnV2.Severity = v.Severity.String() - - // process the CVSS scores if the data is available - if (vuln.CVSS{} != v.CVSSDetails) { - vulnV2.CVE3Score = v.CVSSDetails.ScoreV3 - vulnV2.CVE2Score = v.CVSSDetails.ScoreV2 - vulnV2.CVSS3Vector = v.CVSSDetails.VectorV3 - vulnV2.CVSS2Vector = v.CVSSDetails.VectorV2 - } - if len(v.CWEIds) > 0 { - vulnV2.CWEIDs = strings.Join(v.CWEIds, ",") - } - - // marshall the presented vendor attributes as a json string - if len(v.VendorAttributes) > 0 { - vendorAttributes, err := json.Marshal(v.VendorAttributes) - // set the vendor attributes iff unmarshalling is successful - if err == nil { - vulnV2.VendorAttributes = string(vendorAttributes) - } - } - - _, err = c.dao.InsertForReport(ctx, reportUUID, vulnV2) - if err != nil { - log.Warningf("Could not insert vulnerability record - report: %s, cve_id: %s, scanner: %s, package: %s, package_version: %s", reportUUID, v.ID, registrationUUID, v.Package, v.Version) - } - + cveIDs = append(cveIDs, v.ID) } - log.Infof("Converted %d vulnerability records to the new schema for report ID %s and scanner Id %s", len(vulnReport.Vulnerabilities), reportUUID, registrationUUID) + + records, err := c.dao.List(ctx, q.New(q.KeyWords{"cve_id": q.NewOrList(cveIDs), "registration_uuid": registrationUUID})) + if err != nil { + return err + } + + l := vulnReport.GetVulnerabilityItemList() + s := lib.Set{} + + var ( + outOfDateRecords []*scan.VulnerabilityRecord + recordIDs []int64 + ) + for _, record := range records { + key := record.Key() + + v, ok := l.GetItem(key) + if !ok { + // skip the record which not in the vulnReport.Vulnerabilities + continue + } + + s.Add(key) + + recordIDs = append(recordIDs, record.ID) + + if record.Severity != v.Severity.String() { + record.Severity = v.Severity.String() + outOfDateRecords = append(outOfDateRecords, record) + } + } + + for _, record := range outOfDateRecords { + // Update the severity of the record when it's changed in the scanner, closes #14745 + if err := c.dao.Update(ctx, record, "severity"); err != nil { + return err + } + } + + if len(outOfDateRecords) > 0 { + log.G(ctx).Infof("%d vulnerabilities' severity changed", len(outOfDateRecords)) + } + + var newRecords []*scan.VulnerabilityRecord + for _, v := range vulnReport.Vulnerabilities { + if !s.Exists(v.Key()) { + newRecords = append(newRecords, toVulnerabilityRecord(v, registrationUUID)) + } + } + + for _, record := range newRecords { + recordID, err := c.dao.Create(ctx, record) + if err != nil { + fields := log.Fields{ + "error": err, + "report": reportUUID, + "cveID": record.CVEID, + "package": record.Package, + "packageVersion": record.PackageVersion, + } + log.G(ctx).WithFields(fields).Errorf("Could not insert vulnerability record") + + return err + } + + recordIDs = append(recordIDs, recordID) + } + + if err := c.dao.InsertForReport(ctx, reportUUID, recordIDs...); err != nil { + fields := log.Fields{ + "error": err, + "report": reportUUID, + } + log.G(ctx).WithFields(fields).Errorf("Could not associate vulnerability records to the report") + + return err + } + + fields := log.Fields{ + "report": reportUUID, + "scanner": registrationUUID, + "vulnerabilityRecords": len(vulnReport.Vulnerabilities), + } + log.G(ctx).WithFields(fields).Infof("Converted vulnerability records to the new schema") + return nil } @@ -141,32 +200,9 @@ func (c *nativeToRelationalSchemaConverter) fromSchema(ctx context.Context, repo } vulnerabilityItems := make([]*vuln.VulnerabilityItem, 0) for _, record := range records { - vi := new(vuln.VulnerabilityItem) - vi.ID = record.CVEID - vi.ArtifactDigests = []string{artifactDigest} - vi.CVSSDetails.ScoreV2 = record.CVE2Score - vi.CVSSDetails.ScoreV3 = record.CVE3Score - vi.CVSSDetails.VectorV2 = record.CVSS2Vector - vi.CVSSDetails.VectorV3 = record.CVSS3Vector - cweIDs := strings.Split(record.CWEIDs, ",") - for _, cweID := range cweIDs { - vi.CWEIds = append(vi.CWEIds, cweID) - } - vi.CWEIds = cweIDs - vi.Description = record.Description - vi.FixVersion = record.Fix - vi.Version = record.PackageVersion - urls := strings.Split(record.URLs, "|") - for _, url := range urls { - vi.Links = append(vi.Links, url) - } - vi.Severity = vuln.ParseSeverityVersion3(record.Severity) - vi.Package = record.Package - var vendorAttributes map[string]interface{} - _ = json.Unmarshal([]byte(record.VendorAttributes), &vendorAttributes) - vi.VendorAttributes = vendorAttributes - vulnerabilityItems = append(vulnerabilityItems, vi) + vulnerabilityItems = append(vulnerabilityItems, toVulnerabilityItem(record, artifactDigest)) } + rp := new(vuln.Report) err := json.Unmarshal([]byte(reportSummary), rp) if err != nil { @@ -196,3 +232,73 @@ func (c *nativeToRelationalSchemaConverter) getNativeV1ReportFromResolvedData(ct ctx.GetLogger().Infof("Converted raw data to report. Count of Vulnerabilities in report : %d", len(report.Vulnerabilities)) return report, nil } + +func toVulnerabilityRecord(item *vuln.VulnerabilityItem, registrationUUID string) *scan.VulnerabilityRecord { + record := new(scan.VulnerabilityRecord) + + record.CVEID = item.ID + record.Description = item.Description + record.Package = item.Package + record.PackageVersion = item.Version + record.PackageType = "Unknown" + record.Fix = item.FixVersion + record.URLs = strings.Join(item.Links, "|") + record.RegistrationUUID = registrationUUID + record.Severity = item.Severity.String() + + // process the CVSS scores if the data is available + if (vuln.CVSS{} != item.CVSSDetails) { + record.CVE3Score = item.CVSSDetails.ScoreV3 + record.CVE2Score = item.CVSSDetails.ScoreV2 + record.CVSS3Vector = item.CVSSDetails.VectorV3 + record.CVSS2Vector = item.CVSSDetails.VectorV2 + } + if len(item.CWEIds) > 0 { + record.CWEIDs = strings.Join(item.CWEIds, ",") + } + + // marshall the presented vendor attributes as a json string + if len(item.VendorAttributes) > 0 { + vendorAttributes, err := json.Marshal(item.VendorAttributes) + // set the vendor attributes iff unmarshalling is successful + if err == nil { + record.VendorAttributes = string(vendorAttributes) + } + } + + return record +} + +func toVulnerabilityItem(record *scan.VulnerabilityRecord, artifactDigest string) *vuln.VulnerabilityItem { + item := new(vuln.VulnerabilityItem) + + item.ID = record.CVEID + item.ArtifactDigests = []string{artifactDigest} + item.CVSSDetails.ScoreV2 = record.CVE2Score + item.CVSSDetails.ScoreV3 = record.CVE3Score + item.CVSSDetails.VectorV2 = record.CVSS2Vector + item.CVSSDetails.VectorV3 = record.CVSS3Vector + cweIDs := strings.Split(record.CWEIDs, ",") + for _, cweID := range cweIDs { + item.CWEIds = append(item.CWEIds, cweID) + } + item.CWEIds = cweIDs + item.Description = record.Description + item.FixVersion = record.Fix + item.Version = record.PackageVersion + urls := strings.Split(record.URLs, "|") + for _, url := range urls { + item.Links = append(item.Links, url) + } + item.Severity = vuln.ParseSeverityVersion3(record.Severity) + item.Package = record.Package + var vendorAttributes map[string]interface{} + _ = json.Unmarshal([]byte(record.VendorAttributes), &vendorAttributes) + item.VendorAttributes = vendorAttributes + + return item +} + +func convertingKey(reportUUID string) string { + return fmt.Sprintf("converting:%s", reportUUID) +} diff --git a/src/pkg/scan/report/manager.go b/src/pkg/scan/report/manager.go index 0bddb1898..5baf0b74e 100644 --- a/src/pkg/scan/report/manager.go +++ b/src/pkg/scan/report/manager.go @@ -24,6 +24,11 @@ import ( "github.com/google/uuid" ) +var ( + // Mgr is the global report manager + Mgr = NewManager() +) + // Manager is used to manage the scan reports. type Manager interface { // Create a new report record. diff --git a/src/pkg/scan/vuln/report.go b/src/pkg/scan/vuln/report.go index 535bd0556..0f24737b9 100644 --- a/src/pkg/scan/vuln/report.go +++ b/src/pkg/scan/vuln/report.go @@ -148,6 +148,13 @@ func (l *VulnerabilityItemList) Add(items ...*VulnerabilityItem) { } } +// GetItem returns VulnerabilityItem by key +func (l *VulnerabilityItemList) GetItem(key string) (*VulnerabilityItem, bool) { + item, ok := l.indexed[key] + + return item, ok +} + // GetSeveritySummary returns the severity and summary of l func (l *VulnerabilityItemList) GetSeveritySummary() (Severity, *VulnerabilitySummary) { if l == nil { diff --git a/src/testing/controller/scan/controller.go b/src/testing/controller/scan/controller.go index 6da3949e9..b70e6fc3c 100644 --- a/src/testing/controller/scan/controller.go +++ b/src/testing/controller/scan/controller.go @@ -13,8 +13,6 @@ import ( models "github.com/goharbor/harbor/src/pkg/allowlist/models" - pkgscan "github.com/goharbor/harbor/src/pkg/scan" - scan "github.com/goharbor/harbor/src/controller/scan" ) @@ -177,17 +175,3 @@ func (_m *Controller) ScanAll(ctx context.Context, trigger string, async bool) ( return r0, r1 } - -// UpdateReport provides a mock function with given fields: ctx, report -func (_m *Controller) UpdateReport(ctx context.Context, report *pkgscan.CheckInReport) error { - ret := _m.Called(ctx, report) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, *pkgscan.CheckInReport) error); ok { - r0 = rf(ctx, report) - } else { - r0 = ret.Error(0) - } - - return r0 -}