From a036e4a7b095dcc1543db91a7743fa1505f54bb4 Mon Sep 17 00:00:00 2001 From: Chlins Zhang Date: Mon, 7 Aug 2023 14:00:26 +0800 Subject: [PATCH] fix: skip to delete scan reports if the digest still referenced (#19110) fix: skip to delete scan reports if the digest still referenced by other artifacts Avoid to delete the scan reports in case the artifact deleted but still referenced by the other artifacts. Signed-off-by: chlins --- .../event/handler/internal/artifact.go | 27 ++++++++++++++++--- .../event/handler/internal/artifact_test.go | 7 ++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/controller/event/handler/internal/artifact.go b/src/controller/event/handler/internal/artifact.go index b4428273b..fc89eb6e0 100644 --- a/src/controller/event/handler/internal/artifact.go +++ b/src/controller/event/handler/internal/artifact.go @@ -33,6 +33,8 @@ import ( "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" + pkgArt "github.com/goharbor/harbor/src/pkg/artifact" "github.com/goharbor/harbor/src/pkg/scan/report" "github.com/goharbor/harbor/src/pkg/task" ) @@ -70,6 +72,8 @@ type Handler struct { execMgr task.ExecutionManager // reportMgr for managing scan reports reportMgr report.Manager + // artMgr for managing artifacts + artMgr pkgArt.Manager once sync.Once // pullCountStore caches the pull count group by repository @@ -262,6 +266,7 @@ func (a *Handler) onPush(ctx context.Context, event *event.ArtifactEvent) error func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) error { execMgr := task.ExecMgr reportMgr := report.Mgr + artMgr := pkg.ArtifactMgr // for UT mock if a.execMgr != nil { execMgr = a.execMgr @@ -269,6 +274,9 @@ func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) erro if a.reportMgr != nil { reportMgr = a.reportMgr } + if a.artMgr != nil { + artMgr = a.artMgr + } ids := []int64{event.Artifact.ID} digests := []string{event.Artifact.Digest} @@ -278,7 +286,20 @@ func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) erro digests = append(digests, ref.ChildDigest) } } + // check if the digest also referenced by other artifacts, should exclude it to delete the scan report if still referenced by others. + unrefDigests := []string{} + for _, digest := range digests { + // with the base=* to query all artifacts includes untagged and references + count, err := artMgr.Count(ctx, q.New(q.KeyWords{"digest": digest, "base": "*"})) + if err != nil { + log.Errorf("failed to count the artifact with the digest %s, error: %v", digest, err) + continue + } + if count == 0 { + unrefDigests = append(unrefDigests, digest) + } + } // clean up the scan executions of this artifact and it's references by id log.Debugf("delete the associated scan executions of artifacts %v as the artifacts have been deleted", ids) for _, id := range ids { @@ -288,9 +309,9 @@ func (a *Handler) onDelete(ctx context.Context, event *event.ArtifactEvent) erro } // clean up the scan reports of this artifact and it's references by digest - log.Debugf("delete the associated scan reports of artifacts %v as the artifacts have been deleted", digests) - if err := reportMgr.DeleteByDigests(ctx, digests...); err != nil { - log.Errorf("failed to delete scan reports of artifact %v, error: %v", digests, err) + log.Debugf("delete the associated scan reports of artifacts %v as the artifacts have been deleted", unrefDigests) + if err := reportMgr.DeleteByDigests(ctx, unrefDigests...); err != nil { + log.Errorf("failed to delete scan reports of artifact %v, error: %v", unrefDigests, err) } return nil diff --git a/src/controller/event/handler/internal/artifact_test.go b/src/controller/event/handler/internal/artifact_test.go index 27187760b..23584a55f 100644 --- a/src/controller/event/handler/internal/artifact_test.go +++ b/src/controller/event/handler/internal/artifact_test.go @@ -35,6 +35,8 @@ import ( "github.com/goharbor/harbor/src/pkg/tag" tagmodel "github.com/goharbor/harbor/src/pkg/tag/model/tag" scannerCtlMock "github.com/goharbor/harbor/src/testing/controller/scanner" + "github.com/goharbor/harbor/src/testing/mock" + artMock "github.com/goharbor/harbor/src/testing/pkg/artifact" 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" @@ -50,6 +52,7 @@ type ArtifactHandlerTestSuite struct { scannerCtl scanner.Controller reportMgr *reportMock.Manager execMgr *taskMock.ExecutionManager + artMgr *artMock.Manager } // TestArtifactHandler tests ArtifactHandler. @@ -66,7 +69,8 @@ func (suite *ArtifactHandlerTestSuite) SetupSuite() { suite.scannerCtl = &scannerCtlMock.Controller{} suite.execMgr = &taskMock.ExecutionManager{} suite.reportMgr = &reportMock.Manager{} - suite.handler = &Handler{execMgr: suite.execMgr, reportMgr: suite.reportMgr} + suite.artMgr = &artMock.Manager{} + suite.handler = &Handler{execMgr: suite.execMgr, reportMgr: suite.reportMgr, artMgr: suite.artMgr} // mock artifact _, err := pkg.ArtifactMgr.Create(suite.ctx, &artifact.Artifact{ID: 1, RepositoryID: 1}) @@ -163,6 +167,7 @@ func (suite *ArtifactHandlerTestSuite) TestOnDelete() { suite.execMgr.On("DeleteByVendor", suite.ctx, "IMAGE_SCAN", int64(1)).Return(nil).Times(1) suite.execMgr.On("DeleteByVendor", suite.ctx, "IMAGE_SCAN", int64(2)).Return(nil).Times(1) suite.execMgr.On("DeleteByVendor", suite.ctx, "IMAGE_SCAN", int64(3)).Return(nil).Times(1) + suite.artMgr.On("Count", suite.ctx, mock.Anything).Return(int64(0), nil).Times(3) 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")