Skip to content

Commit

Permalink
Add sbom_report table to store sbom related information
Browse files Browse the repository at this point in the history 
  fixes #20445
  Refactor scan/base_controller.go
  Move MakeReportPlaceholder, GetReportPlaceholder, GetSummary to vul and sbom scanHandler

Signed-off-by: stonezdj <stone.zhang@broadcom.com>
  • Loading branch information
@stonezdj
stonezdj committed May 23, 2024
1 parent 2f4fa29 commit 88b43e3
Show file tree
Hide file tree
Showing 27 changed files with 1,849 additions and 459 deletions.
13 changes: 12 additions & 1 deletion make/migrations/postgresql/0140_2.11.0_schema.up.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@ then set column artifact_type as not null
*/
UPDATE artifact SET artifact_type = media_type WHERE artifact_type IS NULL;

ALTER TABLE artifact ALTER COLUMN artifact_type SET NOT NULL;
ALTER TABLE artifact ALTER COLUMN artifact_type SET NOT NULL;

CREATE TABLE IF NOT EXISTS sbom_report
(
id SERIAL PRIMARY KEY NOT NULL,
uuid VARCHAR(64) UNIQUE NOT NULL,
artifact_id INT NOT NULL,
registration_uuid VARCHAR(64) NOT NULL,
mime_type VARCHAR(256) NOT NULL,
report JSON,
UNIQUE(artifact_id, registration_uuid, mime_type)
);
15 changes: 12 additions & 3 deletions src/controller/event/handler/internal/artifact.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"time"

"github.com/goharbor/harbor/src/controller/artifact"
"github.com/goharbor/harbor/src/controller/artifact/processor/sbom"
sbomprocessor "github.com/goharbor/harbor/src/controller/artifact/processor/sbom"
"github.com/goharbor/harbor/src/controller/event"
"github.com/goharbor/harbor/src/controller/event/operator"
"github.com/goharbor/harbor/src/controller/repository"
Expand All @@ -38,6 +38,7 @@ import (
pkgArt "github.com/goharbor/harbor/src/pkg/artifact"
"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/sbom"
"github.com/goharbor/harbor/src/pkg/task"
)

Expand Down Expand Up @@ -74,6 +75,8 @@ type ArtifactEventHandler struct {
execMgr task.ExecutionManager
// reportMgr for managing scan reports
reportMgr report.Manager
// sbomReportMgr
sbomReportMgr sbom.Manager
// artMgr for managing artifacts
artMgr pkgArt.Manager

Expand Down Expand Up @@ -321,8 +324,14 @@ func (a *ArtifactEventHandler) onDelete(ctx context.Context, event *event.Artifa
log.Errorf("failed to delete scan reports of artifact %v, error: %v", unrefDigests, err)
}

if event.Artifact.Type == sbom.ArtifactTypeSBOM && len(event.Artifact.Digest) > 0 {
if err := reportMgr.DeleteByExtraAttr(ctx, v1.MimeTypeSBOMReport, "sbom_digest", event.Artifact.Digest); err != nil {
// delete sbom_report when the subject artifact is deleted
if err := sbom.Mgr.DeleteByArtifactID(ctx, event.Artifact.ID); err != nil {
log.Errorf("failed to delete scan reports of artifact ID %v, error: %v", event.Artifact.ID, err)
}

Check warning on line 330 in src/controller/event/handler/internal/artifact.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/internal/artifact.go#L329-L330 

Added lines #L329 - L330 were not covered by tests

// delete sbom_report when the accessory artifact is deleted
if event.Artifact.Type == sbomprocessor.ArtifactTypeSBOM && len(event.Artifact.Digest) > 0 {
if err := sbom.Mgr.DeleteByExtraAttr(ctx, v1.MimeTypeSBOMReport, "sbom_digest", event.Artifact.Digest); err != nil {

Check warning on line 334 in src/controller/event/handler/internal/artifact.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/internal/artifact.go#L334 

Added line #L334 was not covered by tests
log.Errorf("failed to delete scan reports of with sbom digest %v, error: %v", event.Artifact.Digest, err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/controller/event/handler/webhook/scan/scan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,15 +144,15 @@ func constructScanImagePayload(ctx context.Context, event *event.ScanImageEvent,

scanSummaries := map[string]interface{}{}
if event.ScanType == v1.ScanTypeVulnerability {
scanSummaries, err = scan.DefaultController.GetSummary(ctx, art, []string{v1.MimeTypeNativeReport, v1.MimeTypeGenericVulnerabilityReport})
scanSummaries, err = scan.DefaultController.GetSummary(ctx, art, event.ScanType, []string{v1.MimeTypeNativeReport, v1.MimeTypeGenericVulnerabilityReport})

Check warning on line 147 in src/controller/event/handler/webhook/scan/scan.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/webhook/scan/scan.go#L147 

Added line #L147 was not covered by tests
if err != nil {
return nil, errors.Wrap(err, "construct scan payload")
}
}

sbomOverview := map[string]interface{}{}
if event.ScanType == v1.ScanTypeSbom {
sbomOverview, err = scan.DefaultController.GetSummary(ctx, art, []string{v1.MimeTypeSBOMReport})
sbomOverview, err = scan.DefaultController.GetSummary(ctx, art, event.ScanType, []string{v1.MimeTypeSBOMReport})

Check warning on line 155 in src/controller/event/handler/webhook/scan/scan.go

View check run for this annotation

Codecov / codecov/patch

src/controller/event/handler/webhook/scan/scan.go#L155 

Added line #L155 was not covered by tests
if err != nil {
return nil, errors.Wrap(err, "construct scan payload")
}
Expand Down
213 changes: 9 additions & 204 deletions src/controller/scan/base_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package scan
import (
"bytes"
"context"
"encoding/json"
"fmt"
"reflect"
"strings"
Expand Down Expand Up @@ -49,7 +48,6 @@ import (
"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"
sbomModel "github.com/goharbor/harbor/src/pkg/scan/sbom/model"
"github.com/goharbor/harbor/src/pkg/scan/vuln"
"github.com/goharbor/harbor/src/pkg/task"
)
Expand Down Expand Up @@ -276,7 +274,8 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti
launchScanJobParams []*launchScanJobParam
)
for _, art := range artifacts {
reports, err := bc.makeReportPlaceholder(ctx, r, art, opts)
handler := sca.GetScanHandler(opts.GetScanType())
reports, err := handler.MakeReportPlaceHolder(ctx, art, r)
if err != nil {
if errors.IsConflictErr(err) {
errs = append(errs, err)
Expand Down Expand Up @@ -566,63 +565,6 @@ func (bc *basicController) startScanAll(ctx context.Context, executionID int64)
return nil
}

func (bc *basicController) makeReportPlaceholder(ctx context.Context, r *scanner.Registration, art *ar.Artifact, opts *Options) ([]*scan.Report, error) {
mimeTypes := r.GetProducesMimeTypes(art.ManifestMediaType, opts.GetScanType())
oldReports, err := bc.manager.GetBy(bc.cloneCtx(ctx), art.Digest, r.UUID, mimeTypes)
if err != nil {
return nil, err
}
if err := bc.deleteArtifactAccessories(ctx, oldReports); err != nil {
return nil, err
}

if err := bc.assembleReports(ctx, oldReports...); err != nil {
return nil, err
}

if len(oldReports) > 0 {
for _, oldReport := range oldReports {
if !job.Status(oldReport.Status).Final() {
return nil, errors.ConflictError(nil).WithMessage("a previous scan process is %s", oldReport.Status)
}
}

for _, oldReport := range oldReports {
if err := bc.manager.Delete(ctx, oldReport.UUID); err != nil {
return nil, err
}
}
}

var reports []*scan.Report

for _, pm := range r.GetProducesMimeTypes(art.ManifestMediaType, opts.GetScanType()) {
report := &scan.Report{
Digest: art.Digest,
RegistrationUUID: r.UUID,
MimeType: pm,
}

create := func(ctx context.Context) error {
reportUUID, err := bc.manager.Create(ctx, report)
if err != nil {
return err
}
report.UUID = reportUUID

return nil
}

if err := orm.WithTransaction(create)(orm.SetTransactionOpNameToContext(ctx, "tx-make-report-placeholder")); err != nil {
return nil, err
}

reports = append(reports, report)
}

return reports, nil
}

// GetReport ...
func (bc *basicController) GetReport(ctx context.Context, artifact *ar.Artifact, mimeTypes []string) ([]*scan.Report, error) {
if artifact == nil {
Expand Down Expand Up @@ -697,95 +639,10 @@ func (bc *basicController) GetReport(ctx context.Context, artifact *ar.Artifact,
return reports, nil
}

func isSBOMMimeTypes(mimeTypes []string) bool {
for _, mimeType := range mimeTypes {
if mimeType == v1.MimeTypeSBOMReport {
return true
}
}
return false
}

// GetSummary ...
func (bc *basicController) GetSummary(ctx context.Context, artifact *ar.Artifact, mimeTypes []string) (map[string]interface{}, error) {
if artifact == nil {
return nil, errors.New("no way to get report summaries for nil artifact")
}
if isSBOMMimeTypes(mimeTypes) {
return bc.GetSBOMSummary(ctx, artifact, mimeTypes)
}
// Get reports first
rps, err := bc.GetReport(ctx, artifact, mimeTypes)
if err != nil {
return nil, err
}

summaries := make(map[string]interface{}, len(rps))
for _, rp := range rps {
sum, err := report.GenerateSummary(rp)
if err != nil {
return nil, err
}

if s, ok := summaries[rp.MimeType]; ok {
r, err := report.MergeSummary(rp.MimeType, s, sum)
if err != nil {
return nil, err
}

summaries[rp.MimeType] = r
} else {
summaries[rp.MimeType] = sum
}
}

return summaries, nil
}

func (bc *basicController) GetSBOMSummary(ctx context.Context, art *ar.Artifact, mimeTypes []string) (map[string]interface{}, error) {
if art == nil {
return nil, errors.New("no way to get report summaries for nil artifact")
}
r, err := bc.sc.GetRegistrationByProject(ctx, art.ProjectID)
if err != nil {
return nil, errors.Wrap(err, "scan controller: get sbom summary")
}
reports, err := bc.manager.GetBy(ctx, art.Digest, r.UUID, mimeTypes)
if err != nil {
return nil, err
}
if len(reports) == 0 {
return map[string]interface{}{}, nil
}
reportContent := reports[0].Report
result := map[string]interface{}{}
if len(reportContent) == 0 {
status := bc.retrieveStatusFromTask(ctx, reports[0].UUID)
if len(status) > 0 {
result[sbomModel.ReportID] = reports[0].UUID
result[sbomModel.ScanStatus] = status
}
log.Debug("no content for current report")
return result, nil
}
err = json.Unmarshal([]byte(reportContent), &result)
return result, err
}

// retrieve the status from task
func (bc *basicController) retrieveStatusFromTask(ctx context.Context, reportID string) string {
if len(reportID) == 0 {
return ""
}
tasks, err := bc.taskMgr.ListScanTasksByReportUUID(ctx, reportID)
if err != nil {
log.Warningf("can not find the task with report UUID %v, error %v", reportID, err)
return ""
}
if len(tasks) > 0 {
return tasks[0].Status
}
return ""
func (bc *basicController) GetSummary(ctx context.Context, artifact *ar.Artifact, scanType string, mimeTypes []string) (map[string]interface{}, error) {
handler := sca.GetScanHandler(scanType)
return handler.GetSummary(ctx, artifact, mimeTypes)

Check warning on line 645 in src/controller/scan/base_controller.go

View check run for this annotation

Codecov / codecov/patch

src/controller/scan/base_controller.go#L643-L645 

Added lines #L643 - L645 were not covered by tests
}

// GetScanLog ...
Expand Down Expand Up @@ -821,7 +678,7 @@ func (bc *basicController) GetScanLog(ctx context.Context, artifact *ar.Artifact
if !scanTaskForArtifacts(t, artifactMap) {
return nil, errors.NotFoundError(nil).WithMessage("scan log with uuid: %s not found", uuid)
}
for _, reportUUID := range getReportUUIDs(t.ExtraAttrs) {
for _, reportUUID := range GetReportUUIDs(t.ExtraAttrs) {
reportUUIDToTasks[reportUUID] = t
}
}
Expand Down Expand Up @@ -902,14 +759,6 @@ func scanTaskForArtifacts(task *task.Task, artifactMap map[int64]interface{}) bo
return exist
}

// DeleteReports ...
func (bc *basicController) DeleteReports(ctx context.Context, digests ...string) error {
if err := bc.manager.DeleteByDigests(ctx, digests...); err != nil {
return errors.Wrap(err, "scan controller: delete reports")
}
return nil
}

func (bc *basicController) GetVulnerable(ctx context.Context, artifact *ar.Artifact, allowlist allowlist.CVESet, allowlistIsExpired bool) (*Vulnerable, error) {
if artifact == nil {
return nil, errors.New("no way to get vulnerable for nil artifact")
Expand Down Expand Up @@ -1204,7 +1053,7 @@ func (bc *basicController) assembleReports(ctx context.Context, reports ...*scan

reportUUIDToTasks := map[string]*task.Task{}
for _, task := range tasks {
for _, reportUUID := range getReportUUIDs(task.ExtraAttrs) {
for _, reportUUID := range GetReportUUIDs(task.ExtraAttrs) {
reportUUIDToTasks[reportUUID] = task
}
}
Expand Down Expand Up @@ -1275,7 +1124,8 @@ func getArtifactTag(extraAttrs map[string]interface{}) string {
return tag
}

func getReportUUIDs(extraAttrs map[string]interface{}) []string {
// GetReportUUIDs returns the report UUIDs from the extra attributes
func GetReportUUIDs(extraAttrs map[string]interface{}) []string {
var reportUUIDs []string

if extraAttrs != nil {
Expand Down Expand Up @@ -1314,48 +1164,3 @@ func parseOptions(options ...Option) (*Options, error) {

return ops, nil
}

// deleteArtifactAccessories delete the accessory in reports, only delete sbom accessory
func (bc *basicController) deleteArtifactAccessories(ctx context.Context, reports []*scan.Report) error {
for _, rpt := range reports {
if rpt.MimeType != v1.MimeTypeSBOMReport {
continue
}
if err := bc.deleteArtifactAccessory(ctx, rpt.Report); err != nil {
return err
}
}
return nil
}

// deleteArtifactAccessory check if current report has accessory info, if there is, delete it
func (bc *basicController) deleteArtifactAccessory(ctx context.Context, report string) error {
if len(report) == 0 {
return nil
}
sbomSummary := sbomModel.Summary{}
if err := json.Unmarshal([]byte(report), &sbomSummary); err != nil {
// it could be a non sbom report, just skip
log.Debugf("fail to unmarshal %v, skip to delete sbom report", err)
return nil
}
repo, dgst := sbomSummary.SBOMAccArt()
if len(repo) == 0 || len(dgst) == 0 {
return nil
}
art, err := bc.ar.GetByReference(ctx, repo, dgst, nil)
if err != nil {
if errors.IsNotFoundErr(err) {
return nil
}
return err
}
if art == nil {
return nil
}
err = bc.ar.Delete(ctx, art.ID)
if errors.IsNotFoundErr(err) {
return nil
}
return err
}
Loading

0 comments on commit 88b43e3

Please sign in to comment.