Skip to content

Commit

Permalink
fix: clean up scan executions and reports after deleting artifact
Browse files Browse the repository at this point in the history
Cleanup the associated resources(scan executions and scan reports) after
deletion of artifact.

Fixes: goharbor#18634

Signed-off-by: chlins <chenyuzh@vmware.com>
  • Loading branch information
chlins committed May 31, 2023
1 parent e19ec96 commit 33de2de
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 1 deletion.
3 changes: 3 additions & 0 deletions src/controller/artifact/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/controller/event/handler/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
39 changes: 39 additions & 0 deletions src/controller/event/handler/internal/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down Expand Up @@ -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$<projectName>+<Scanner UUID (8byte)>-<Scanner Name>-<UUID>`
// verify it by the prefix `robot$<projectName>+<Scanner UUID (8byte)>`
Expand Down
16 changes: 15 additions & 1 deletion src/controller/event/handler/internal/artifact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -46,6 +48,8 @@ type ArtifactHandlerTestSuite struct {
handler *Handler
projectManager project.Manager
scannerCtl scanner.Controller
reportMgr *reportMock.Manager
execMgr *taskMock.ExecutionManager
}

// TestArtifactHandler tests ArtifactHandler.
Expand All @@ -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})
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 33de2de

Please sign in to comment.