From 1b955cd28efde8c52c88d83454e6cabeb9c29c84 Mon Sep 17 00:00:00 2001 From: He Weiwei Date: Tue, 8 Jun 2021 10:11:10 +0000 Subject: [PATCH] fix: supply the latest tag of artifact to scan request Closes #14416 #14299 Signed-off-by: He Weiwei --- .../event/handler/internal/artifact.go | 2 +- src/controller/event/handler/internal/util.go | 9 +- .../event/handler/internal/util_test.go | 19 +++ .../event/handler/webhook/scan/scan.go | 4 +- src/controller/scan/base_controller.go | 121 +++++++++++++----- src/controller/scan/base_controller_test.go | 7 + src/controller/scan/callback.go | 1 + src/controller/scan/options.go | 12 +- src/server/v2.0/handler/scan.go | 8 +- 9 files changed, 145 insertions(+), 38 deletions(-) diff --git a/src/controller/event/handler/internal/artifact.go b/src/controller/event/handler/internal/artifact.go index fa4b039c7..efb7d6222 100644 --- a/src/controller/event/handler/internal/artifact.go +++ b/src/controller/event/handler/internal/artifact.go @@ -91,7 +91,7 @@ func (a *Handler) addPullCount(ctx context.Context, event *event.ArtifactEvent) func (a *Handler) onPush(ctx context.Context, event *event.ArtifactEvent) error { go func() { - if err := autoScan(ctx, &artifact.Artifact{Artifact: *event.Artifact}); err != nil { + if err := autoScan(ctx, &artifact.Artifact{Artifact: *event.Artifact}, event.Tags...); err != nil { log.Errorf("scan artifact %s@%s failed, error: %v", event.Artifact.RepositoryName, event.Artifact.Digest, err) } }() diff --git a/src/controller/event/handler/internal/util.go b/src/controller/event/handler/internal/util.go index 5f7a1cde3..5a5e1b110 100644 --- a/src/controller/event/handler/internal/util.go +++ b/src/controller/event/handler/internal/util.go @@ -24,7 +24,7 @@ import ( ) // autoScan scan artifact when the project of the artifact enable auto scan -func autoScan(ctx context.Context, a *artifact.Artifact) error { +func autoScan(ctx context.Context, a *artifact.Artifact, tags ...string) error { proj, err := project.Ctl.Get(ctx, a.ProjectID) if err != nil { return err @@ -35,6 +35,11 @@ func autoScan(ctx context.Context, a *artifact.Artifact) error { // transaction here to work with the image index return orm.WithTransaction(func(ctx context.Context) error { - return scan.DefaultController.Scan(ctx, a) + options := []scan.Option{} + if len(tags) > 0 { + options = append(options, scan.WithTag(tags[0])) + } + + return scan.DefaultController.Scan(ctx, a, options...) })(ctx) } diff --git a/src/controller/event/handler/internal/util_test.go b/src/controller/event/handler/internal/util_test.go index fb018d140..95e53eeb4 100644 --- a/src/controller/event/handler/internal/util_test.go +++ b/src/controller/event/handler/internal/util_test.go @@ -18,10 +18,12 @@ import ( "testing" "github.com/goharbor/harbor/src/controller/artifact" + "github.com/goharbor/harbor/src/controller/event" "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/controller/scan" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/orm" + pkg "github.com/goharbor/harbor/src/pkg/artifact" proModels "github.com/goharbor/harbor/src/pkg/project/models" projecttesting "github.com/goharbor/harbor/src/testing/controller/project" scantesting "github.com/goharbor/harbor/src/testing/controller/scan" @@ -107,6 +109,23 @@ func (suite *AutoScanTestSuite) TestAutoScanFailed() { suite.Error(autoScan(ctx, art)) } +func (suite *AutoScanTestSuite) TestWithArtifactEvent() { + mock.OnAnything(suite.projectController, "Get").Return(&proModels.Project{ + Metadata: map[string]string{ + proModels.ProMetaAutoScan: "true", + }, + }, nil) + + mock.OnAnything(suite.scanController, "Scan").Return(nil) + + event := &event.ArtifactEvent{ + Artifact: &pkg.Artifact{}, + } + + ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{}) + suite.Nil(autoScan(ctx, &artifact.Artifact{Artifact: *event.Artifact}, event.Tags...)) +} + func TestAutoScanTestSuite(t *testing.T) { suite.Run(t, &AutoScanTestSuite{}) } diff --git a/src/controller/event/handler/webhook/scan/scan.go b/src/controller/event/handler/webhook/scan/scan.go index 713adb6b6..d8f2bcfa3 100644 --- a/src/controller/event/handler/webhook/scan/scan.go +++ b/src/controller/event/handler/webhook/scan/scan.go @@ -110,9 +110,9 @@ func constructScanImagePayload(event *event.ScanImageEvent, project *proModels.P Operator: event.Operator, } - reference := event.Artifact.Digest + reference := event.Artifact.Tag if reference == "" { - reference = event.Artifact.Tag + reference = event.Artifact.Digest } resURL, err := util.BuildImageResourceURL(event.Artifact.Repository, reference) diff --git a/src/controller/scan/base_controller.go b/src/controller/scan/base_controller.go index 45255c09b..7d2b04067 100644 --- a/src/controller/scan/base_controller.go +++ b/src/controller/scan/base_controller.go @@ -26,6 +26,7 @@ import ( ar "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/controller/robot" sc "github.com/goharbor/harbor/src/controller/scanner" + "github.com/goharbor/harbor/src/controller/tag" "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/config" @@ -61,6 +62,7 @@ const ( registrationKey = "registration" artifactIDKey = "artifact_id" + artifactTagKey = "artifact_tag" reportUUIDsKey = "report_uuids" robotIDKey = "robot_id" ) @@ -77,6 +79,15 @@ type uuidGenerator func() (string, error) // utility methods. type configGetter func(cfg string) (string, error) +// launchScanJobParam is a param to launch scan job. +type launchScanJobParam struct { + ExecutionID int64 + Registration *scanner.Registration + Artifact *ar.Artifact + Tag string + Reports []*scan.Report +} + // basicController is default implementation of api.Controller interface type basicController struct { // Manage the scan report records @@ -87,6 +98,8 @@ type basicController struct { sc sc.Controller // Robot account controller rc robot.Controller + // Tag controller + tagCtl tag.Controller // UUID generator uuid uuidGenerator // Configuration getter func @@ -112,6 +125,8 @@ func NewController() Controller { sc: sc.DefaultController, // Refer to the default robot account controller rc: robot.Ctl, + // Refer to the default tag controller + tagCtl: tag.Ctl, // Generate UUID with uuid lib uuid: func() (string, error) { aUUID, err := uuid.NewUUID() @@ -210,14 +225,16 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti return errors.BadRequestError(nil).WithMessage("the configured scanner %s does not support scanning artifact with mime type %s", r.Name, artifact.ManifestMediaType) } - type Param struct { - Artifact *ar.Artifact - Reports []*scan.Report + // Parse options + opts, err := parseOptions(options...) + if err != nil { + return errors.Wrap(err, "scan controller: scan") } - params := []*Param{} - - var errs []error + var ( + errs []error + launchScanJobParams []*launchScanJobParam + ) for _, art := range artifacts { reports, err := bc.makeReportPlaceholder(ctx, r, art) if err != nil { @@ -228,8 +245,27 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti } } + var tag string + if art.Digest == artifact.Digest { + tag = opts.Tag + } + + if tag == "" { + latestTag, err := bc.getLatestTagOfArtifact(ctx, art.ID) + if err != nil { + return err + } + + tag = latestTag + } + if len(reports) > 0 { - params = append(params, &Param{Artifact: art, Reports: reports}) + launchScanJobParams = append(launchScanJobParams, &launchScanJobParam{ + Registration: r, + Artifact: art, + Tag: tag, + Reports: reports, + }) } } @@ -238,12 +274,6 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti return errs[0] } - // Parse options - opts, err := parseOptions(options...) - if err != nil { - return errors.Wrap(err, "scan controller: scan") - } - if opts.ExecutionID == 0 { extraAttrs := map[string]interface{}{ artfiactKey: map[string]interface{}{ @@ -266,15 +296,17 @@ func (bc *basicController) Scan(ctx context.Context, artifact *ar.Artifact, opti } errs = errs[:0] - for _, param := range params { - if err := bc.launchScanJob(ctx, opts.ExecutionID, param.Artifact, r, param.Reports); err != nil { + for _, launchScanJobParam := range launchScanJobParams { + launchScanJobParam.ExecutionID = opts.ExecutionID + + if err := bc.launchScanJob(ctx, launchScanJobParam); err != nil { log.G(ctx).Warningf("scan artifact %s@%s failed, error: %v", artifact.RepositoryName, artifact.Digest, err) errs = append(errs, err) } } // all scanning of the artifacts failed - if len(errs) == len(params) { + if len(errs) == len(launchScanJobParams) { return fmt.Errorf("scan artifact %s@%s failed", artifact.RepositoryName, artifact.Digest) } @@ -795,14 +827,14 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64 } // launchScanJob launches a job to run scan -func (bc *basicController) launchScanJob(ctx context.Context, executionID int64, artifact *ar.Artifact, registration *scanner.Registration, reports []*scan.Report) error { +func (bc *basicController) launchScanJob(ctx context.Context, param *launchScanJobParam) error { // don't launch scan job for the artifact which is not supported by the scanner - if !hasCapability(registration, artifact) { + if !hasCapability(param.Registration, param.Artifact) { return nil } var ck string - if registration.UseInternalAddr { + if param.Registration.UseInternalAddr { ck = configCoreInternalAddr } else { ck = configRegistryEndpoint @@ -813,7 +845,7 @@ func (bc *basicController) launchScanJob(ctx context.Context, executionID int64, return errors.Wrap(err, "scan controller: launch scan job") } - robot, err := bc.makeRobotAccount(ctx, artifact.ProjectID, artifact.RepositoryName, registration) + robot, err := bc.makeRobotAccount(ctx, param.Artifact.ProjectID, param.Artifact.RepositoryName, param.Registration) if err != nil { return errors.Wrap(err, "scan controller: launch scan job") } @@ -824,14 +856,15 @@ func (bc *basicController) launchScanJob(ctx context.Context, executionID int64, URL: registryAddr, }, Artifact: &v1.Artifact{ - NamespaceID: artifact.ProjectID, - Repository: artifact.RepositoryName, - Digest: artifact.Digest, - MimeType: artifact.ManifestMediaType, + NamespaceID: param.Artifact.ProjectID, + Repository: param.Artifact.RepositoryName, + Digest: param.Artifact.Digest, + Tag: param.Tag, + MimeType: param.Artifact.ManifestMediaType, }, } - rJSON, err := registration.ToJSON() + rJSON, err := param.Registration.ToJSON() if err != nil { return errors.Wrap(err, "scan controller: launch scan job") } @@ -846,16 +879,16 @@ func (bc *basicController) launchScanJob(ctx context.Context, executionID int64, return errors.Wrap(err, "launch scan job") } - mimes := make([]string, len(reports)) - reportUUIDs := make([]string, len(reports)) - for i, report := range reports { + mimes := make([]string, len(param.Reports)) + reportUUIDs := make([]string, len(param.Reports)) + for i, report := range param.Reports { mimes[i] = report.MimeType reportUUIDs[i] = report.UUID } params := make(map[string]interface{}) params[sca.JobParamRegistration] = rJSON - params[sca.JobParameterAuthType] = registration.GetRegistryAuthorizationType() + params[sca.JobParameterAuthType] = param.Registration.GetRegistryAuthorizationType() params[sca.JobParameterRequest] = sJSON params[sca.JobParameterMimes] = mimes params[sca.JobParameterRobot] = robotJSON @@ -871,7 +904,8 @@ func (bc *basicController) launchScanJob(ctx context.Context, executionID int64, // keep the report uuids in array so that when ?| operator support by the FilterRaw method of beego's orm // we can list the tasks of the scan reports by one SQL extraAttrs := map[string]interface{}{ - artifactIDKey: artifact.ID, + artifactIDKey: param.Artifact.ID, + artifactTagKey: param.Tag, robotIDKey: robot.ID, reportUUIDsKey: reportUUIDs, } @@ -885,7 +919,7 @@ func (bc *basicController) launchScanJob(ctx context.Context, executionID int64, extraAttrs["report:"+reportUUID] = "1" } - _, err = bc.taskMgr.Create(ctx, executionID, j, extraAttrs) + _, err = bc.taskMgr.Create(ctx, param.ExecutionID, j, extraAttrs) return err } @@ -983,6 +1017,20 @@ func (bc *basicController) assembleReports(ctx context.Context, reports ...*scan return nil } +func (bc *basicController) getLatestTagOfArtifact(ctx context.Context, artifactID int64) (string, error) { + query := q.New(q.KeyWords{"artifact_id": artifactID}) + tags, err := bc.tagCtl.List(ctx, query.First(q.NewSort("push_time", true)), nil) + if err != nil { + return "", err + } + + if len(tags) == 0 { + return "", nil + } + + return tags[0].Name, nil +} + func getArtifactID(extraAttrs map[string]interface{}) int64 { var artifactID float64 if extraAttrs != nil { @@ -994,6 +1042,17 @@ func getArtifactID(extraAttrs map[string]interface{}) int64 { return int64(artifactID) } +func getArtifactTag(extraAttrs map[string]interface{}) string { + var tag string + if extraAttrs != nil { + if v, ok := extraAttrs[artifactTagKey]; ok { + tag, _ = v.(string) + } + } + + return tag +} + func getReportUUIDs(extraAttrs map[string]interface{}) []string { var reportUUIDs []string diff --git a/src/controller/scan/base_controller_test.go b/src/controller/scan/base_controller_test.go index ce77059c0..d8da439a3 100644 --- a/src/controller/scan/base_controller_test.go +++ b/src/controller/scan/base_controller_test.go @@ -42,6 +42,7 @@ import ( artifacttesting "github.com/goharbor/harbor/src/testing/controller/artifact" robottesting "github.com/goharbor/harbor/src/testing/controller/robot" scannertesting "github.com/goharbor/harbor/src/testing/controller/scanner" + tagtesting "github.com/goharbor/harbor/src/testing/controller/tag" ormtesting "github.com/goharbor/harbor/src/testing/lib/orm" "github.com/goharbor/harbor/src/testing/mock" postprocessorstesting "github.com/goharbor/harbor/src/testing/pkg/scan/postprocessors" @@ -59,6 +60,8 @@ type ControllerTestSuite struct { artifactCtl *artifacttesting.Controller originalArtifactCtl artifact.Controller + tagCtl *tagtesting.FakeController + registration *scanner.Registration artifact *artifact.Artifact rawReport string @@ -255,6 +258,9 @@ func (suite *ControllerTestSuite) SetupSuite() { suite.ar = &artifacttesting.Controller{} + suite.tagCtl = &tagtesting.FakeController{} + suite.tagCtl.On("List", mock.Anything, mock.Anything, mock.Anything).Return(nil, nil) + suite.execMgr = &tasktesting.ExecutionManager{} suite.taskMgr = &tasktesting.Manager{} @@ -264,6 +270,7 @@ func (suite *ControllerTestSuite) SetupSuite() { ar: suite.ar, sc: sc, rc: rc, + tagCtl: suite.tagCtl, uuid: func() (string, error) { return "the-uuid-123", nil }, diff --git a/src/controller/scan/callback.go b/src/controller/scan/callback.go index 631a338e7..9bfbf4093 100644 --- a/src/controller/scan/callback.go +++ b/src/controller/scan/callback.go @@ -92,6 +92,7 @@ func scanTaskStatusChange(ctx context.Context, taskID int64, status string) (err NamespaceID: art.ProjectID, Repository: art.RepositoryName, Digest: art.Digest, + Tag: getArtifactTag(t.ExtraAttrs), MimeType: art.ManifestMediaType, }, Status: status, diff --git a/src/controller/scan/options.go b/src/controller/scan/options.go index fef599ca0..a87644833 100644 --- a/src/controller/scan/options.go +++ b/src/controller/scan/options.go @@ -16,7 +16,8 @@ package scan // Options keep the settings/configurations for scanning. type Options struct { - ExecutionID int64 // The execution id to scan artifact + ExecutionID int64 // The execution id to scan artifact + Tag string // The tag of the artifact to scan } // Option represents an option item by func template. @@ -34,3 +35,12 @@ func WithExecutionID(executionID int64) Option { return nil } } + +// WithTag sets the tag option. +func WithTag(tag string) Option { + return func(options *Options) error { + options.Tag = tag + + return nil + } +} diff --git a/src/server/v2.0/handler/scan.go b/src/server/v2.0/handler/scan.go index efdc22f39..c5ffc85da 100644 --- a/src/server/v2.0/handler/scan.go +++ b/src/server/v2.0/handler/scan.go @@ -23,6 +23,7 @@ import ( "github.com/goharbor/harbor/src/controller/artifact" "github.com/goharbor/harbor/src/controller/scan" "github.com/goharbor/harbor/src/lib/errors" + "github.com/goharbor/harbor/src/pkg/distribution" operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/scan" ) @@ -58,7 +59,12 @@ func (s *scanAPI) ScanArtifact(ctx context.Context, params operation.ScanArtifac return s.SendError(ctx, err) } - if err := s.scanCtl.Scan(ctx, artifact); err != nil { + options := []scan.Option{} + if !distribution.IsDigest(params.Reference) { + options = append(options, scan.WithTag(params.Reference)) + } + + if err := s.scanCtl.Scan(ctx, artifact, options...); err != nil { return s.SendError(ctx, err) }