From 33de2dec5e58c825f18bb7152876c76da2efa3b3 Mon Sep 17 00:00:00 2001 From: chlins Date: Wed, 17 May 2023 14:25:24 +0800 Subject: [PATCH] fix: clean up scan executions and reports after deleting artifact Cleanup the associated resources(scan executions and scan reports) after deletion of artifact. Fixes: #18634 Signed-off-by: chlins --- src/controller/artifact/controller.go | 3 ++ src/controller/event/handler/init.go | 1 + .../event/handler/internal/artifact.go | 39 +++++++++++++++++++ .../event/handler/internal/artifact_test.go | 16 +++++++- 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/controller/artifact/controller.go b/src/controller/artifact/controller.go index 5a11a8e1ce60..6b1683730225 100644 --- a/src/controller/artifact/controller.go +++ b/src/controller/artifact/controller.go @@ -76,6 +76,9 @@ var ( } ) +// OnDeleteFunc is the function for running after deleting artifact +type OnDeleteFunc func(ctx context.Context, art *Artifact) error + // Controller defines the operations related with artifacts and tags type Controller interface { // Ensure the artifact specified by the digest exists under the repository, diff --git a/src/controller/event/handler/init.go b/src/controller/event/handler/init.go index b74c924ddb26..06dda44d510c 100644 --- a/src/controller/event/handler/init.go +++ b/src/controller/event/handler/init.go @@ -70,6 +70,7 @@ func init() { // internal _ = notifier.Subscribe(event.TopicPullArtifact, &internal.Handler{}) _ = notifier.Subscribe(event.TopicPushArtifact, &internal.Handler{}) + _ = notifier.Subscribe(event.TopicDeleteArtifact, &internal.Handler{}) _ = task.RegisterTaskStatusChangePostFunc(job.ReplicationVendorType, func(ctx context.Context, taskID int64, status string) error { notification.AddEvent(ctx, &metadata.ReplicationMetaData{ diff --git a/src/controller/event/handler/internal/artifact.go b/src/controller/event/handler/internal/artifact.go index 284934d5ece9..d07f2f5e4dc1 100644 --- a/src/controller/event/handler/internal/artifact.go +++ b/src/controller/event/handler/internal/artifact.go @@ -27,10 +27,13 @@ import ( "github.com/goharbor/harbor/src/controller/event" "github.com/goharbor/harbor/src/controller/repository" "github.com/goharbor/harbor/src/controller/tag" + "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" + "github.com/goharbor/harbor/src/pkg/scan/report" + "github.com/goharbor/harbor/src/pkg/task" ) const ( @@ -62,6 +65,11 @@ func init() { // Handler preprocess artifact event data type Handler struct { + // execMgr for managing executions + execMgr task.ExecutionManager + // reportMgr for managing scan reports + reportMgr report.Manager + once sync.Once // pullCountStore caches the pull count group by repository // map[repositoryID]counts @@ -87,6 +95,8 @@ func (a *Handler) Handle(ctx context.Context, value interface{}) error { return a.onPull(ctx, v.ArtifactEvent) case *event.PushArtifactEvent: return a.onPush(ctx, v.ArtifactEvent) + case *event.DeleteArtifactEvent: + return a.onDelete(ctx, v.ArtifactEvent) default: log.Errorf("Can not handler this event type! %#v", v) } @@ -244,6 +254,35 @@ func (a *Handler) onPush(ctx context.Context, event *event.ArtifactEvent) error return nil } +func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) error { + execMgr := task.ExecMgr + reportMgr := report.Mgr + // for UT mock + if a.execMgr != nil { + execMgr = a.execMgr + } + if a.reportMgr != nil { + reportMgr = a.reportMgr + } + + // clean up the scan executions of this artifact by id + if err := execMgr.DeleteByVendor(ctx, job.ImageScanJobVendorType, event.Artifact.ID); err != nil { + log.Errorf("failed to delete scan executions of artifact %d, error: %v", event.Artifact.ID, err) + } + // clean up the scan reports of this artifact and it's references by digest + digests := []string{event.Artifact.Digest} + if len(event.Artifact.References) > 0 { + for _, ref := range event.Artifact.References { + digests = append(digests, ref.ChildDigest) + } + } + if err := reportMgr.DeleteByDigests(ctx, digests...); err != nil { + log.Errorf("failed to delete scan reports of artifact %v, error: %v", digests, err) + } + + return nil +} + // isScannerUser check if the current user is a scanner user by its prefix // usually a scanner user should be named like `robot$+--` // verify it by the prefix `robot$+` diff --git a/src/controller/event/handler/internal/artifact_test.go b/src/controller/event/handler/internal/artifact_test.go index 1a2a56af7f3c..9824ec7f20f9 100644 --- a/src/controller/event/handler/internal/artifact_test.go +++ b/src/controller/event/handler/internal/artifact_test.go @@ -36,6 +36,8 @@ import ( tagmodel "github.com/goharbor/harbor/src/pkg/tag/model/tag" scannerCtlMock "github.com/goharbor/harbor/src/testing/controller/scanner" projectMock "github.com/goharbor/harbor/src/testing/pkg/project" + reportMock "github.com/goharbor/harbor/src/testing/pkg/scan/report" + taskMock "github.com/goharbor/harbor/src/testing/pkg/task" ) // ArtifactHandlerTestSuite is test suite for artifact handler. @@ -46,6 +48,8 @@ type ArtifactHandlerTestSuite struct { handler *Handler projectManager project.Manager scannerCtl scanner.Controller + reportMgr *reportMock.Manager + execMgr *taskMock.ExecutionManager } // TestArtifactHandler tests ArtifactHandler. @@ -57,10 +61,12 @@ func TestArtifactHandler(t *testing.T) { func (suite *ArtifactHandlerTestSuite) SetupSuite() { common_dao.PrepareTestForPostgresSQL() config.Init() - suite.handler = &Handler{} suite.ctx = orm.NewContext(context.TODO(), beegoorm.NewOrm()) suite.projectManager = &projectMock.Manager{} suite.scannerCtl = &scannerCtlMock.Controller{} + suite.execMgr = &taskMock.ExecutionManager{} + suite.reportMgr = &reportMock.Manager{} + suite.handler = &Handler{execMgr: suite.execMgr, reportMgr: suite.reportMgr} // mock artifact _, err := pkg.ArtifactMgr.Create(suite.ctx, &artifact.Artifact{ID: 1, RepositoryID: 1}) @@ -152,6 +158,14 @@ func (suite *ArtifactHandlerTestSuite) TestOnPull() { }, 3*asyncFlushDuration, asyncFlushDuration/2, "wait for pull_count async update") } +func (suite *ArtifactHandlerTestSuite) TestOnDelete() { + evt := &event.ArtifactEvent{Artifact: &artifact.Artifact{ID: 1, RepositoryID: 1, Digest: "mock-digest", References: []*artifact.Reference{{ChildDigest: "ref-1"}, {ChildDigest: "ref-2"}}}} + suite.execMgr.On("DeleteByVendor", suite.ctx, "IMAGE_SCAN", int64(1)).Return(nil).Times(1) + suite.reportMgr.On("DeleteByDigests", suite.ctx, "mock-digest", "ref-1", "ref-2").Return(nil).Times(1) + err := suite.handler.onDelete(suite.ctx, evt) + suite.Nil(err, "onDelete should return nil") +} + func (suite *ArtifactHandlerTestSuite) TestIsScannerUser() { type args struct { prefix string