fix: event-based replication deletion not work when policy with label…

… filter Fix event-based replication deletion on remote registry not triggered when the replication policy configured the label filter. Signed-off-by: chlins <[email protected]>
goharbor · Nov 22, 2024 · 41576dd · 41576dd
1 parent 66c98c8
commit 41576dd
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 34 deletions.
diff --git a/src/controller/artifact/controller.go b/src/controller/artifact/controller.go
@@ -326,7 +326,7 @@ func (c *controller) Delete(ctx context.Context, id int64) error {
 // the error handling logic for the root parent artifact and others is different
 // "isAccessory" is used to specify whether the artifact is an accessory.
 func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot, isAccessory bool) error {
-	art, err := c.Get(ctx, id, &Option{WithTag: true, WithAccessory: true})
+	art, err := c.Get(ctx, id, &Option{WithTag: true, WithAccessory: true, WithLabel: true})
 	if err != nil {
 		// return nil if the nonexistent artifact isn't the root parent
 		if !isRoot && errors.IsErr(err, errors.NotFoundCode) {
@@ -450,14 +450,20 @@ func (c *controller) deleteDeeply(ctx context.Context, id int64, isRoot, isAcces
 
 	// only fire event for the root parent artifact
 	if isRoot {
-		var tags []string
+		var tags, labels []string
 		for _, tag := range art.Tags {
 			tags = append(tags, tag.Name)
 		}
+
+		for _, label := range art.Labels {
+			labels = append(labels, label.Name)
+		}
+
 		notification.AddEvent(ctx, &metadata.DeleteArtifactEventMetadata{
 			Ctx:      ctx,
 			Artifact: &art.Artifact,
 			Tags:     tags,
+			Labels:   labels,
 		})
 	}
 

diff --git a/src/controller/artifact/controller_test.go b/src/controller/artifact/controller_test.go
@@ -487,6 +487,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 	// root artifact and doesn't exist
 	c.artMgr.On("Get", mock.Anything, mock.Anything).Return(nil, errors.NotFoundError(nil))
 	c.accMgr.On("List", mock.Anything, mock.Anything).Return([]accessorymodel.Accessory{}, nil)
+	c.labelMgr.On("ListByArtifact", mock.Anything, mock.Anything).Return([]*model.Label{}, nil)
 	err := c.ctl.deleteDeeply(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, true, false)
 	c.Require().NotNil(err)
 	c.Assert().True(errors.IsErr(err, errors.NotFoundCode))
@@ -497,6 +498,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 	// child artifact and doesn't exist
 	c.artMgr.On("Get", mock.Anything, mock.Anything).Return(nil, errors.NotFoundError(nil))
 	c.accMgr.On("List", mock.Anything, mock.Anything).Return([]accessorymodel.Accessory{}, nil)
+	c.labelMgr.On("ListByArtifact", mock.Anything, mock.Anything).Return([]*model.Label{}, nil)
 	err = c.ctl.deleteDeeply(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, false, false)
 	c.Require().Nil(err)
 
@@ -516,6 +518,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 	c.repoMgr.On("Get", mock.Anything, mock.Anything).Return(&repomodel.RepoRecord{}, nil)
 	c.artrashMgr.On("Create", mock.Anything, mock.Anything).Return(int64(0), nil)
 	c.accMgr.On("List", mock.Anything, mock.Anything).Return([]accessorymodel.Accessory{}, nil)
+	c.labelMgr.On("ListByArtifact", mock.Anything, mock.Anything).Return([]*model.Label{}, nil)
 	err = c.ctl.deleteDeeply(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, false, false)
 	c.Require().Nil(err)
 
@@ -532,6 +535,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 		},
 	}, nil)
 	c.accMgr.On("List", mock.Anything, mock.Anything).Return([]accessorymodel.Accessory{}, nil)
+	c.labelMgr.On("ListByArtifact", mock.Anything, mock.Anything).Return([]*model.Label{}, nil)
 	err = c.ctl.deleteDeeply(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, true, false)
 	c.Require().NotNil(err)
 
@@ -548,6 +552,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 		},
 	}, nil)
 	c.accMgr.On("List", mock.Anything, mock.Anything).Return([]accessorymodel.Accessory{}, nil)
+	c.labelMgr.On("ListByArtifact", mock.Anything, mock.Anything).Return([]*model.Label{}, nil)
 	err = c.ctl.deleteDeeply(nil, 1, false, false)
 	c.Require().Nil(err)
 
@@ -573,6 +578,7 @@ func (c *controllerTestSuite) TestDeleteDeeply() {
 	c.blobMgr.On("CleanupAssociationsForProject", mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	c.repoMgr.On("Get", mock.Anything, mock.Anything).Return(&repomodel.RepoRecord{}, nil)
 	c.artrashMgr.On("Create", mock.Anything, mock.Anything).Return(int64(0), nil)
+	c.labelMgr.On("ListByArtifact", mock.Anything, mock.Anything).Return([]*model.Label{}, nil)
 	err = c.ctl.deleteDeeply(orm.NewContext(nil, &ormtesting.FakeOrmer{}), 1, true, true)
 	c.Require().Nil(err)
 

diff --git a/src/controller/event/handler/replication/replication.go b/src/controller/event/handler/replication/replication.go
@@ -23,7 +23,6 @@ import (
 	"github.com/goharbor/harbor/src/controller/project"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/orm"
-	"github.com/goharbor/harbor/src/pkg/label"
 	labmodel "github.com/goharbor/harbor/src/pkg/label/model"
 	"github.com/goharbor/harbor/src/pkg/reg/model"
 )
@@ -82,12 +81,6 @@ func (r *Handler) handlePushArtifact(ctx context.Context, event *event.PushArtif
 		return err
 	}
 	public = prj.IsPublic()
-	// list attached labels
-	labels, err := label.Mgr.ListByArtifact(ctx, art.ID)
-	if err != nil {
-		log.Errorf("failed to list artifact %d labels, error: %v", art.ID, err)
-		return err
-	}
 
 	e := &repevent.Event{
 		Type: repevent.EventTypeArtifactPush,
@@ -105,7 +98,7 @@ func (r *Handler) handlePushArtifact(ctx context.Context, event *event.PushArtif
 						Type:   art.Type,
 						Digest: art.Digest,
 						Tags:   event.Tags,
-						Labels: abstractLabelNames(labels),
+						Labels: event.Labels,
 					}},
 			},
 		},
@@ -116,13 +109,6 @@ func (r *Handler) handlePushArtifact(ctx context.Context, event *event.PushArtif
 
 func (r *Handler) handleDeleteArtifact(ctx context.Context, event *event.DeleteArtifactEvent) error {
 	art := event.Artifact
-	// list attached labels
-	labels, err := label.Mgr.ListByArtifact(ctx, art.ID)
-	if err != nil {
-		log.Errorf("failed to list artifact %d labels, error: %v", art.ID, err)
-		return err
-	}
-
 	e := &repevent.Event{
 		Type: repevent.EventTypeArtifactDelete,
 		Resource: &model.Resource{
@@ -136,7 +122,7 @@ func (r *Handler) handleDeleteArtifact(ctx context.Context, event *event.DeleteA
 						Type:   art.Type,
 						Digest: art.Digest,
 						Tags:   event.Tags,
-						Labels: abstractLabelNames(labels),
+						Labels: event.Labels,
 					}},
 			},
 			Deleted: true,
@@ -155,12 +141,6 @@ func (r *Handler) handleCreateTag(ctx context.Context, event *event.CreateTagEve
 		return err
 	}
 	public = prj.IsPublic()
-	// list attached labels
-	labels, err := label.Mgr.ListByArtifact(ctx, art.ID)
-	if err != nil {
-		log.Errorf("failed to list artifact %d labels, error: %v", art.ID, err)
-		return err
-	}
 
 	e := &repevent.Event{
 		Type: repevent.EventTypeArtifactPush,
@@ -178,7 +158,7 @@ func (r *Handler) handleCreateTag(ctx context.Context, event *event.CreateTagEve
 						Type:   art.Type,
 						Digest: art.Digest,
 						Tags:   []string{event.Tag},
-						Labels: abstractLabelNames(labels),
+						Labels: event.Labels,
 					}},
 			},
 		},
@@ -189,12 +169,6 @@ func (r *Handler) handleCreateTag(ctx context.Context, event *event.CreateTagEve
 
 func (r *Handler) handleDeleteTag(ctx context.Context, event *event.DeleteTagEvent) error {
 	art := event.AttachedArtifact
-	// list attached labels
-	labels, err := label.Mgr.ListByArtifact(ctx, art.ID)
-	if err != nil {
-		log.Errorf("failed to list artifact %d labels, error: %v", art.ID, err)
-		return err
-	}
 
 	e := &repevent.Event{
 		Type: repevent.EventTypeTagDelete,
@@ -209,7 +183,7 @@ func (r *Handler) handleDeleteTag(ctx context.Context, event *event.DeleteTagEve
 						Type:   art.Type,
 						Digest: art.Digest,
 						Tags:   []string{event.Tag},
-						Labels: abstractLabelNames(labels),
+						Labels: event.Labels,
 					}},
 			},
 			Deleted:     true,

diff --git a/src/controller/event/metadata/artifact.go b/src/controller/event/metadata/artifact.go
@@ -29,6 +29,7 @@ type PushArtifactEventMetadata struct {
 	Ctx      context.Context
 	Artifact *artifact.Artifact
 	Tag      string
+	Labels   []string
 }
 
 // Resolve to the event from the metadata
@@ -37,6 +38,7 @@ func (p *PushArtifactEventMetadata) Resolve(event *event.Event) error {
 		EventType:  event2.TopicPushArtifact,
 		Repository: p.Artifact.RepositoryName,
 		Artifact:   p.Artifact,
+		Labels:     p.Labels,
 		OccurAt:    time.Now(),
 	}
 	if p.Tag != "" {
@@ -86,6 +88,7 @@ type DeleteArtifactEventMetadata struct {
 	Ctx      context.Context
 	Artifact *artifact.Artifact
 	Tags     []string
+	Labels   []string
 }
 
 // Resolve to the event from the metadata
@@ -96,6 +99,7 @@ func (d *DeleteArtifactEventMetadata) Resolve(event *event.Event) error {
 			Repository: d.Artifact.RepositoryName,
 			Artifact:   d.Artifact,
 			Tags:       d.Tags,
+			Labels:     d.Labels,
 			OccurAt:    time.Now(),
 		},
 	}

diff --git a/src/controller/event/metadata/tag.go b/src/controller/event/metadata/tag.go
@@ -28,6 +28,7 @@ import (
 type CreateTagEventMetadata struct {
 	Ctx              context.Context
 	Tag              string
+	Labels           []string
 	AttachedArtifact *artifact.Artifact
 }
 
@@ -37,6 +38,7 @@ func (c *CreateTagEventMetadata) Resolve(event *event.Event) error {
 		EventType:        event2.TopicCreateTag,
 		Repository:       c.AttachedArtifact.RepositoryName,
 		Tag:              c.Tag,
+		Labels:           c.Labels,
 		AttachedArtifact: c.AttachedArtifact,
 		OccurAt:          time.Now(),
 	}
@@ -53,6 +55,7 @@ func (c *CreateTagEventMetadata) Resolve(event *event.Event) error {
 type DeleteTagEventMetadata struct {
 	Ctx              context.Context
 	Tag              string
+	Labels           []string
 	AttachedArtifact *artifact.Artifact
 }
 
@@ -62,6 +65,7 @@ func (d *DeleteTagEventMetadata) Resolve(event *event.Event) error {
 		EventType:        event2.TopicDeleteTag,
 		Repository:       d.AttachedArtifact.RepositoryName,
 		Tag:              d.Tag,
+		Labels:           d.Labels,
 		AttachedArtifact: d.AttachedArtifact,
 		OccurAt:          time.Now(),
 	}

diff --git a/src/controller/event/topic.go b/src/controller/event/topic.go
@@ -137,6 +137,7 @@ type ArtifactEvent struct {
 	Repository string
 	Artifact   *artifact.Artifact
 	Tags       []string // when the artifact is pushed by digest, the tag here will be null
+	Labels     []string
 	Operator   string
 	OccurAt    time.Time
 }
@@ -238,6 +239,7 @@ type CreateTagEvent struct {
 	EventType        string
 	Repository       string
 	Tag              string
+	Labels           []string
 	AttachedArtifact *artifact.Artifact
 	Operator         string
 	OccurAt          time.Time
@@ -266,6 +268,7 @@ type DeleteTagEvent struct {
 	EventType        string
 	Repository       string
 	Tag              string
+	Labels           []string
 	AttachedArtifact *artifact.Artifact
 	Operator         string
 	OccurAt          time.Time

diff --git a/src/server/v2.0/handler/artifact.go b/src/server/v2.0/handler/artifact.go
@@ -238,7 +238,8 @@ func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagP
 
 	art, err := a.artCtl.GetByReference(ctx, fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName),
 		params.Reference, &artifact.Option{
-			WithTag: true,
+			WithTag:   true,
+			WithLabel: true,
 		})
 	if err != nil {
 		return a.SendError(ctx, err)
@@ -252,10 +253,16 @@ func (a *artifactAPI) CreateTag(ctx context.Context, params operation.CreateTagP
 		return a.SendError(ctx, err)
 	}
 
+	var labels []string
+	for _, label := range art.Labels {
+		labels = append(labels, label.Name)
+	}
+
 	// fire event
 	notification.AddEvent(ctx, &metadata.CreateTagEventMetadata{
 		Ctx:              ctx,
 		Tag:              tag.Name,
+		Labels:           labels,
 		AttachedArtifact: &art.Artifact,
 	})
 
@@ -281,7 +288,8 @@ func (a *artifactAPI) DeleteTag(ctx context.Context, params operation.DeleteTagP
 	}
 	artifact, err := a.artCtl.GetByReference(ctx, fmt.Sprintf("%s/%s", params.ProjectName, params.RepositoryName),
 		params.Reference, &artifact.Option{
-			WithTag: true,
+			WithTag:   true,
+			WithLabel: true,
 		})
 	if err != nil {
 		return a.SendError(ctx, err)
@@ -303,10 +311,16 @@ func (a *artifactAPI) DeleteTag(ctx context.Context, params operation.DeleteTagP
 		return a.SendError(ctx, err)
 	}
 
+	var labels []string
+	for _, label := range artifact.Labels {
+		labels = append(labels, label.Name)
+	}
+
 	// fire event
 	notification.AddEvent(ctx, &metadata.DeleteTagEventMetadata{
 		Ctx:              ctx,
 		Tag:              params.TagName,
+		Labels:           labels,
 		AttachedArtifact: &artifact.Artifact,
 	})