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

…#21215) 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 <chlins.zhang@gmail.com>
goharbor · Nov 26, 2024 · 05233b0 · 05233b0
1 parent 4a12623
commit 05233b0
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 45 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/artifact/model.go b/src/controller/artifact/model.go
@@ -94,6 +94,16 @@ func (artifact *Artifact) SetSBOMAdditionLink(sbomDgst string, version string) {
 	artifact.AdditionLinks[addition] = &AdditionLink{HREF: href, Absolute: false}
 }
 
+// AbstractLabelNames abstracts the label names from the artifact.
+func (artifact *Artifact) AbstractLabelNames() []string {
+	var names []string
+	for _, label := range artifact.Labels {
+		names = append(names, label.Name)
+	}
+
+	return names
+}
+
 // AdditionLink is a link via that the addition can be fetched
 type AdditionLink struct {
 	HREF     string `json:"href"`

diff --git a/src/controller/artifact/model_test.go b/src/controller/artifact/model_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/goharbor/harbor/src/pkg/accessory/model/cosign"
+	"github.com/goharbor/harbor/src/pkg/label/model"
 )
 
 func TestUnmarshalJSONWithACC(t *testing.T) {
@@ -104,3 +105,58 @@ func TestUnmarshalJSONWithPartial(t *testing.T) {
 	assert.Equal(t, "", artifact.Type)
 	assert.Equal(t, "application/vnd.docker.container.image.v1+json", artifact.MediaType)
 }
+
+func TestAbstractLabelNames(t *testing.T) {
+	tests := []struct {
+		name     string
+		artifact Artifact
+		want     []string
+	}{
+		{
+			name: "Nil labels",
+			artifact: Artifact{
+				Labels: nil,
+			},
+			want: []string{},
+		},
+		{
+			name: "Single label",
+			artifact: Artifact{
+				Labels: []*model.Label{
+					{Name: "label1"},
+				},
+			},
+			want: []string{"label1"},
+		},
+		{
+			name: "Multiple labels",
+			artifact: Artifact{
+				Labels: []*model.Label{
+					{Name: "label1"},
+					{Name: "label2"},
+					{Name: "label3"},
+				},
+			},
+			want: []string{"label1", "label2", "label3"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.artifact.AbstractLabelNames()
+
+			// Check if lengths match
+			if len(got) != len(tt.want) {
+				t.Errorf("AbstractLabelNames() got length = %v, want length = %v", len(got), len(tt.want))
+				return
+			}
+
+			// Check if elements match
+			for i := range got {
+				if got[i] != tt.want[i] {
+					t.Errorf("AbstractLabelNames() got[%d] = %v, want[%d] = %v", i, got[i], i, tt.want[i])
+				}
+			}
+		})
+	}
+}
diff --git a/src/controller/event/handler/replication/replication.go b/src/controller/event/handler/replication/replication.go
@@ -23,8 +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"
 )
 
@@ -63,16 +61,6 @@ func (r *Handler) IsStateful() bool {
 	return false
 }
 
-// abstractLabelNames returns labels name.
-func abstractLabelNames(labels []*labmodel.Label) []string {
-	res := make([]string, 0, len(labels))
-	for _, lab := range labels {
-		res = append(res, lab.Name)
-	}
-
-	return res
-}
-
 func (r *Handler) handlePushArtifact(ctx context.Context, event *event.PushArtifactEvent) error {
 	art := event.Artifact
 	public := false
@@ -82,12 +70,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 +87,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 +98,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 +111,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 +130,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 +147,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 +158,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 +172,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