Fix replication filter bug

This commit fixes the bug of replication filter, see #13593 for more detail
Fixes #13593

Signed-off-by: Wenkai Yin <yinw@vmware.com>

src/controller/replication/flow/copy.go

@@ -51,14 +51,12 @@ func (c *copyFlow) Run(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	var srcResources []*model.Resource
-	if len(c.resources) > 0 {
-		srcResources, err = filterResources(c.resources, c.policy.Filters)
-	} else {
+	srcResources := c.resources
+	if len(srcResources) == 0 {
 		srcResources, err = fetchResources(srcAdapter, c.policy)
-	}
-	if err != nil {
-		return err
+		if err != nil {
+			return err
+		}
 	}
 
 	isStopped, err := c.isExecutionStopped(ctx)

src/controller/replication/flow/deletion.go

@@ -19,7 +19,6 @@ import (
 	"encoding/json"
 
 	"github.com/goharbor/harbor/src/jobservice/job"
-	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/task"
 	"github.com/goharbor/harbor/src/replication/model"
 )
@@ -45,20 +44,7 @@ func NewDeletionFlow(executionID int64, policy *model.Policy, resources ...*mode
 }
 
 func (d *deletionFlow) Run(ctx context.Context) error {
-	logger := log.GetLogger(ctx)
-	srcResources, err := filterResources(d.resources, d.policy.Filters)
-	if err != nil {
-		return err
-	}
-	if len(srcResources) == 0 {
-		// no candidates, mark the execution as done directly
-		if err := d.executionMgr.MarkDone(ctx, d.executionID, "no resources need to be replicated"); err != nil {
-			logger.Errorf("failed to mark done for the execution %d: %v", d.executionID, err)
-		}
-		return nil
-	}
-
-	srcResources = assembleSourceResources(srcResources, d.policy)
+	srcResources := assembleSourceResources(d.resources, d.policy)
 	dstResources := assembleDestinationResources(srcResources, d.policy)
 
 	return d.createTasks(ctx, srcResources, dstResources)

src/controller/replication/flow/stage.go

@@ -19,7 +19,6 @@ import (
 
 	"github.com/goharbor/harbor/src/lib/log"
 	adp "github.com/goharbor/harbor/src/replication/adapter"
-	"github.com/goharbor/harbor/src/replication/filter"
 	"github.com/goharbor/harbor/src/replication/model"
 	"github.com/goharbor/harbor/src/replication/util"
 )
@@ -110,16 +109,6 @@ func fetchResources(adapter adp.Adapter, policy *model.Policy) ([]*model.Resourc
 	return resources, nil
 }
 
-// apply the filters to the resources and returns the filtered resources
-func filterResources(resources []*model.Resource, filters []*model.Filter) ([]*model.Resource, error) {
-	resources, err := filter.DoFilterResources(resources, filters)
-	if err != nil {
-		return nil, err
-	}
-	log.Debug("filter resources completed")
-	return resources, nil
-}
-
 // assemble the source resources by filling the registry information
 func assembleSourceResources(resources []*model.Resource,
 	policy *model.Policy) []*model.Resource {

src/controller/replication/flow/stage_test.go

@@ -67,83 +67,6 @@ func (s *stageTestSuite) TestFetchResources() {
 	adapter.AssertExpectations(s.T())
 }
 
-func (s *stageTestSuite) TestFilterResources() {
-	resources := []*model.Resource{
-		{
-			Type: model.ResourceTypeImage,
-			Metadata: &model.ResourceMetadata{
-				Repository: &model.Repository{
-					Name: "library/hello-world",
-				},
-				Artifacts: []*model.Artifact{
-					{
-						Tags: []string{"latest"},
-					},
-				},
-			},
-			Deleted:  true,
-			Override: true,
-		},
-		{
-			Type: model.ResourceTypeChart,
-			Metadata: &model.ResourceMetadata{
-				Repository: &model.Repository{
-					Name: "library/harbor",
-				},
-				Artifacts: []*model.Artifact{
-					{
-						Tags: []string{"0.2.0"},
-					},
-					{
-						Tags: []string{"0.3.0"},
-					},
-				},
-			},
-			Deleted:  true,
-			Override: true,
-		},
-		{
-			Type: model.ResourceTypeChart,
-			Metadata: &model.ResourceMetadata{
-				Repository: &model.Repository{
-					Name: "library/mysql",
-				},
-				Artifacts: []*model.Artifact{
-					{
-						Tags: []string{"1.0"},
-					},
-				},
-			},
-			Deleted:  true,
-			Override: true,
-		},
-	}
-	filters := []*model.Filter{
-		{
-			Type:  model.FilterTypeResource,
-			Value: model.ResourceTypeChart,
-		},
-		{
-			Type:  model.FilterTypeName,
-			Value: "library/*",
-		},
-		{
-			Type:  model.FilterTypeName,
-			Value: "library/harbor",
-		},
-		{
-			Type:  model.FilterTypeTag,
-			Value: "0.2.?",
-		},
-	}
-	res, err := filterResources(resources, filters)
-	s.Require().Nil(err)
-	s.Len(res, 1)
-	s.Equal("library/harbor", res[0].Metadata.Repository.Name)
-	s.Equal(1, len(res[0].Metadata.Artifacts))
-	s.Equal("0.2.0", res[0].Metadata.Artifacts[0].Tags[0])
-}
-
 func (s *stageTestSuite) TestAssembleSourceResources() {
 	resources := []*model.Resource{
 		{

src/replication/event/handler.go

@@ -17,17 +17,17 @@ package event
 import (
 	"errors"
 	"fmt"
-	"github.com/goharbor/harbor/src/lib/orm"
-	"github.com/goharbor/harbor/src/pkg/task"
 
 	commonthttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/controller/replication"
 	"github.com/goharbor/harbor/src/lib/log"
+	"github.com/goharbor/harbor/src/lib/orm"
+	"github.com/goharbor/harbor/src/pkg/task"
 	"github.com/goharbor/harbor/src/replication/config"
+	"github.com/goharbor/harbor/src/replication/filter"
 	"github.com/goharbor/harbor/src/replication/model"
 	"github.com/goharbor/harbor/src/replication/policy"
 	"github.com/goharbor/harbor/src/replication/registry"
-	"github.com/goharbor/harbor/src/replication/util"
 )
 
 // Handler is the handler to handle event
@@ -115,39 +115,21 @@ func (h *handler) getRelatedPolicies(resource *model.Resource) ([]*model.Policy,
 		if resource.Deleted && !policy.Deletion {
 			continue
 		}
-		// doesn't match the name filter
-		m, err := match(policy.Filters, resource)
+
+		resources, err := filter.DoFilterResources([]*model.Resource{resource}, policy.Filters)
 		if err != nil {
 			return nil, err
 		}
-		if !m {
+		// doesn't match the filters
+		if len(resources) == 0 {
 			continue
 		}
+
 		result = append(result, policy)
 	}
 	return result, nil
 }
 
-// TODO unify the match logic with other?
-func match(filters []*model.Filter, resource *model.Resource) (bool, error) {
-	match := true
-	repository := resource.Metadata.Repository.Name
-	for _, filter := range filters {
-		if filter.Type != model.FilterTypeName {
-			continue
-		}
-		m, err := util.Match(filter.Value.(string), repository)
-		if err != nil {
-			return false, err
-		}
-		if !m {
-			match = false
-			break
-		}
-	}
-	return match, nil
-}
-
 // PopulateRegistries populates the source registry and destination registry properties for policy
 func PopulateRegistries(registryMgr registry.Manager, policy *model.Policy) error {
 	if policy == nil {

src/replication/event/handler_test.go

@@ -190,6 +190,13 @@ func TestGetRelatedPolicies(t *testing.T) {
 			Repository: &model.Repository{
 				Name: "library/hello-world",
 			},
+			Artifacts: []*model.Artifact{
+				{
+					Type:   "image",
+					Digest: "sha256:90659bf80b44ce6be8234e6ff90a1ac34acbeb826903b02cfa0da11c82cbc042",
+					Tags:   []string{"latest"},
+				},
+			},
 		},
 	})
 	require.Nil(t, err)
@@ -202,6 +209,13 @@ func TestGetRelatedPolicies(t *testing.T) {
 			Repository: &model.Repository{
 				Name: "library/hello-world",
 			},
+			Artifacts: []*model.Artifact{
+				{
+					Type:   "image",
+					Digest: "sha256:90659bf80b44ce6be8234e6ff90a1ac34acbeb826903b02cfa0da11c82cbc042",
+					Tags:   []string{"latest"},
+				},
+			},
 		},
 		Deleted: true,
 	})

src/replication/filter/resource.go

@@ -56,5 +56,34 @@ func DoFilterResources(resources []*model.Resource, filters []*model.Filter) ([]
 			Override:     resource.Override,
 		})
 	}
+
+	// remove this after we deprecate chart museum
+	return filterByResourceType(result, filters)
+}
+
+// After we deprecated chart museum, the resource types model.ResourceTypeArtifact and model.ResourceTypeChart
+// are useless, this function should be removed as well
+func filterByResourceType(resources []*model.Resource, filters []*model.Filter) ([]*model.Resource, error) {
+	var resourceType model.ResourceType
+	for _, filter := range filters {
+		if filter.Type == model.FilterTypeResource {
+			// model.ResourceTypeImage is handled by artifact filters in function "DoFilterResources"
+			if filter.Value.(model.ResourceType) == model.ResourceTypeArtifact || filter.Value.(model.ResourceType) == model.ResourceTypeChart {
+				resourceType = filter.Value.(model.ResourceType)
+			}
+			break
+		}
+	}
+	// no resource type, return the candidates directly
+	if len(resourceType) == 0 {
+		return resources, nil
+	}
+
+	var result []*model.Resource
+	for _, resource := range resources {
+		if resource.Type == resourceType {
+			result = append(result, resource)
+		}
+	}
 	return result, nil
 }