Merge pull request #5003 from ywk253100/180517_label_filter_api

Add label filter in replication policy API

docs/swagger.yaml

@@ -2959,9 +2959,14 @@ definitions:
         description: >-
           The replication policy filter kind. The valid values are project,
           repository and tag.
+      value:
+        type: 
+          - string
+          - integer
+        description: The value of replication policy filter. When creating repository and tag filter, filling it with the pattern as string. When creating label filter, filling it with label ID as integer.
       pattern:
         type: string
-        description: The replication policy filter pattern.
+        description: Depraceted, use value instead. The replication policy filter pattern.
       metadata:
         type: object
         description: This map object is the replication policy filter metadata.
@@ -3541,7 +3546,7 @@ definitions:
         type: string
         description: The color of label.
       scope:
-        type: integer
+        type: string
         description: The scope of label, g for global labels and p for project labels.
       project_id:
         type: integer

src/replication/models/filter.go

@@ -30,13 +30,44 @@ type Filter struct {
 
 // Valid ...
 func (f *Filter) Valid(v *validation.Validation) {
-	if !(f.Kind == replication.FilterItemKindProject ||
-		f.Kind == replication.FilterItemKindRepository ||
-		f.Kind == replication.FilterItemKindTag) {
-		v.SetError("kind", fmt.Sprintf("invalid filter kind: %s", f.Kind))
-	}
-
+	switch f.Kind {
+	case replication.FilterItemKindProject,
+		replication.FilterItemKindRepository,
+		replication.FilterItemKindTag:
+		if f.Value == nil {
+			// check the Filter.Pattern if the Filter.Value is nil for compatibility
 			if len(f.Pattern) == 0 {
-		v.SetError("pattern", "filter pattern can not be empty")
+				v.SetError("value", "the value can not be empty")
+			}
+			return
+		}
+		pattern, ok := f.Value.(string)
+		if !ok {
+			v.SetError("value", "the type of value should be string for project, repository and image filter")
+			return
+		}
+		if len(pattern) == 0 {
+			v.SetError("value", "the value can not be empty")
+			return
+		}
+	case replication.FilterItemKindLabel:
+		if f.Value == nil {
+			v.SetError("value", "the value can not be empty")
+			return
+		}
+		labelID, ok := f.Value.(float64)
+		i := int64(labelID)
+		if !ok || float64(i) != labelID {
+			v.SetError("value", "the type of value should be integer for label filter")
+			return
+		}
+		if i <= 0 {
+			v.SetError("value", fmt.Sprintf("invalid label ID: %d", i))
+			return
+		}
+		f.Value = i
+	default:
+		v.SetError("kind", fmt.Sprintf("invalid filter kind: %s", f.Kind))
+		return
 	}
 }

src/replication/models/filter_test.go

@@ -35,6 +35,29 @@ func TestValid(t *testing.T) {
 			Kind:    replication.FilterItemKindRepository,
 			Pattern: "*",
 		}: false,
+		&Filter{
+			Kind:  replication.FilterItemKindRepository,
+			Value: "*",
+		}: false,
+		&Filter{
+			Kind: replication.FilterItemKindLabel,
+		}: true,
+		&Filter{
+			Kind:  replication.FilterItemKindLabel,
+			Value: "",
+		}: true,
+		&Filter{
+			Kind:  replication.FilterItemKindLabel,
+			Value: 1.2,
+		}: true,
+		&Filter{
+			Kind:  replication.FilterItemKindLabel,
+			Value: -1,
+		}: true,
+		&Filter{
+			Kind:  replication.FilterItemKindLabel,
+			Value: 1,
+		}: true,
 	}
 
 	for filter, hasError := range cases {

src/replication/policy/manager.go

@@ -20,6 +20,7 @@ import (
 
 	"github.com/vmware/harbor/src/common/dao"
 	persist_models "github.com/vmware/harbor/src/common/models"
+	"github.com/vmware/harbor/src/replication"
 	"github.com/vmware/harbor/src/replication/models"
 	"github.com/vmware/harbor/src/ui/config"
 )
@@ -110,6 +111,11 @@ func convertFromPersistModel(policy *persist_models.RepPolicy) (models.Replicati
 			if filters[i].Value == nil && len(filters[i].Pattern) > 0 {
 				filters[i].Value = filters[i].Pattern
 			}
+			// convert the type of Value to int64 as the default type of
+			// json Unmarshal for number is float64
+			if filters[i].Kind == replication.FilterItemKindLabel {
+				filters[i].Value = int64(filters[i].Value.(float64))
+			}
 		}
 		ply.Filters = filters
 	}

src/ui/api/models/replication_policy.go

@@ -56,11 +56,13 @@ func (r *ReplicationPolicy) Valid(v *validation.Validation) {
 		v.SetError("targets", "can not be empty")
 	}
 
-	for _, filter := range r.Filters {
-		filter.Valid(v)
+	for i := range r.Filters {
+		r.Filters[i].Valid(v)
 	}
 
-	if r.Trigger != nil {
+	if r.Trigger == nil {
+		v.SetError("trigger", "can not be empty")
+	} else {
 		r.Trigger.Valid(v)
 	}
 }

src/ui/api/replication_policy.go

@@ -23,6 +23,7 @@ import (
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/models"
 	"github.com/vmware/harbor/src/common/utils/log"
+	"github.com/vmware/harbor/src/replication"
 	"github.com/vmware/harbor/src/replication/core"
 	rep_models "github.com/vmware/harbor/src/replication/models"
 	api_models "github.com/vmware/harbor/src/ui/api/models"
@@ -34,6 +35,14 @@ type RepPolicyAPI struct {
 	BaseController
 }
 
+// labelWrapper wraps models.Label by adding the property "inactive"
+type labelWrapper struct {
+	models.Label
+	// if the label referenced by label filter is deleted,
+	// inactive will set be true
+	Inactive bool `json:"inactive"`
+}
+
 // Prepare validates whether the user has system admin role
 func (pa *RepPolicyAPI) Prepare() {
 	pa.BaseController.Prepare()
@@ -153,6 +162,22 @@ func (pa *RepPolicyAPI) Post() {
 		}
 	}
 
+	// check the existence of labels
+	for _, filter := range policy.Filters {
+		if filter.Kind == replication.FilterItemKindLabel {
+			labelID := filter.Value.(int64)
+			label, err := dao.GetLabel(labelID)
+			if err != nil {
+				pa.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", labelID, err))
+				return
+			}
+			if label == nil {
+				pa.HandleNotFound(fmt.Sprintf("label %d not found", labelID))
+				return
+			}
+		}
+	}
+
 	id, err := core.GlobalController.CreatePolicy(convertToRepPolicy(policy))
 	if err != nil {
 		pa.HandleInternalServerError(fmt.Sprintf("failed to create policy: %v", err))
@@ -219,6 +244,22 @@ func (pa *RepPolicyAPI) Put() {
 		}
 	}
 
+	// check the existence of labels
+	for _, filter := range policy.Filters {
+		if filter.Kind == replication.FilterItemKindLabel {
+			labelID := filter.Value.(int64)
+			label, err := dao.GetLabel(labelID)
+			if err != nil {
+				pa.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", labelID, err))
+				return
+			}
+			if label == nil {
+				pa.HandleNotFound(fmt.Sprintf("label %d not found", labelID))
+				return
+			}
+		}
+	}
+
 	if err = core.GlobalController.UpdatePolicy(convertToRepPolicy(policy)); err != nil {
 		pa.HandleInternalServerError(fmt.Sprintf("failed to update policy %d: %v", id, err))
 		return
@@ -279,7 +320,6 @@ func convertFromRepPolicy(projectMgr promgr.ProjectManager, policy rep_models.Re
 		ID:                policy.ID,
 		Name:              policy.Name,
 		Description:       policy.Description,
-		Filters:           policy.Filters,
 		ReplicateDeletion: policy.ReplicateDeletion,
 		Trigger:           policy.Trigger,
 		CreationTime:      policy.CreationTime,
@@ -306,6 +346,28 @@ func convertFromRepPolicy(projectMgr promgr.ProjectManager, policy rep_models.Re
 		ply.Targets = append(ply.Targets, target)
 	}
 
+	// populate label used in label filter
+	for _, filter := range policy.Filters {
+		if filter.Kind == replication.FilterItemKindLabel {
+			labelID := filter.Value.(int64)
+			label, err := dao.GetLabel(labelID)
+			if err != nil {
+				return nil, err
+			}
+			lw := &labelWrapper{}
+			// if the label is not found, set inactive to true
+			if label == nil {
+				lw.ID = labelID
+				lw.Name = "unknown"
+				lw.Inactive = true
+			} else {
+				lw.Label = *label
+			}
+			filter.Value = lw
+		}
+		ply.Filters = append(ply.Filters, filter)
+	}
+
 	// TODO call the method from replication controller
 	errJobCount, err := dao.GetTotalCountOfRepJobs(&models.RepJobQuery{
 		PolicyID: policy.ID,

src/ui/api/replication_policy_test.go

@@ -37,6 +37,7 @@ var (
 	projectID            int64 = 1
 	targetID             int64
 	policyID             int64
+	labelID2             int64
 )
 
 func TestRepPolicyAPIPost(t *testing.T) {
@@ -52,6 +53,14 @@ func TestRepPolicyAPIPost(t *testing.T) {
 	CommonAddTarget()
 	targetID = int64(CommonGetTarget())
 
+	var err error
+	labelID2, err = dao.AddLabel(&models.Label{
+		Name:  "label_for_replication_filter",
+		Scope: "g",
+	})
+	require.Nil(t, err)
+	defer dao.DeleteLabel(labelID2)
+
 	cases := []*codeCheckingCase{
 		// 401
 		&codeCheckingCase{
@@ -231,6 +240,41 @@ func TestRepPolicyAPIPost(t *testing.T) {
 			},
 			code: http.StatusNotFound,
 		},
+		// 404, label not found
+		&codeCheckingCase{
+			request: &testingRequest{
+				method: http.MethodPost,
+				url:    repPolicyAPIBasePath,
+				bodyJSON: &api_models.ReplicationPolicy{
+					Name: policyName,
+					Projects: []*models.Project{
+						&models.Project{
+							ProjectID: projectID,
+						},
+					},
+					Targets: []*models.RepTarget{
+						&models.RepTarget{
+							ID: targetID,
+						},
+					},
+					Filters: []rep_models.Filter{
+						rep_models.Filter{
+							Kind:    replication.FilterItemKindRepository,
+							Pattern: "*",
+						},
+						rep_models.Filter{
+							Kind:  replication.FilterItemKindLabel,
+							Value: 10000,
+						},
+					},
+					Trigger: &rep_models.Trigger{
+						Kind: replication.TriggerKindManual,
+					},
+				},
+				credential: sysAdmin,
+			},
+			code: http.StatusNotFound,
+		},
 		// 201
 		&codeCheckingCase{
 			request: &testingRequest{
@@ -253,6 +297,10 @@ func TestRepPolicyAPIPost(t *testing.T) {
 							Kind:    replication.FilterItemKindRepository,
 							Pattern: "*",
 						},
+						rep_models.Filter{
+							Kind:  replication.FilterItemKindLabel,
+							Value: labelID2,
+						},
 					},
 					Trigger: &rep_models.Trigger{
 						Kind: replication.TriggerKindManual,
@@ -303,6 +351,21 @@ func TestRepPolicyAPIGet(t *testing.T) {
 	require.Nil(t, err)
 	assert.Equal(t, policyID, policy.ID)
 	assert.Equal(t, policyName, policy.Name)
+	assert.Equal(t, 2, len(policy.Filters))
+	found := false
+	for _, filter := range policy.Filters {
+		if filter.Kind == replication.FilterItemKindLabel {
+			found = true
+			label, ok := filter.Value.(map[string]interface{})
+			if assert.True(t, ok) {
+				id := int64(label["id"].(float64))
+				inactive := label["inactive"].(bool)
+				assert.Equal(t, labelID2, id)
+				assert.True(t, inactive)
+			}
+		}
+	}
+	assert.True(t, found)
 }
 
 func TestRepPolicyAPIList(t *testing.T) {