From ad62fffb0ef7e6052a36d4c520870d05ddf0c416 Mon Sep 17 00:00:00 2001 From: Wenkai Yin 79628 Date: Thu, 17 May 2018 15:45:41 +0800 Subject: [PATCH] Add label filter in replication policy API The label filter can be used to filter images when do the replication, this commit provides the filter in replication policy --- docs/swagger.yaml | 9 +++- src/replication/models/filter.go | 45 ++++++++++++++--- src/replication/models/filter_test.go | 23 +++++++++ src/replication/policy/manager.go | 6 +++ src/ui/api/models/replication_policy.go | 8 ++-- src/ui/api/replication_policy.go | 64 ++++++++++++++++++++++++- src/ui/api/replication_policy_test.go | 63 ++++++++++++++++++++++++ 7 files changed, 205 insertions(+), 13 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index d1eea120d..784f656b8 100644 --- a/docs/swagger.yaml +++ b/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 diff --git a/src/replication/models/filter.go b/src/replication/models/filter.go index 149df0954..e2b2a704d 100644 --- a/src/replication/models/filter.go +++ b/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) { + 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("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)) - } - - if len(f.Pattern) == 0 { - v.SetError("pattern", "filter pattern can not be empty") + return } } diff --git a/src/replication/models/filter_test.go b/src/replication/models/filter_test.go index 4026f0e20..5be9b5dad 100644 --- a/src/replication/models/filter_test.go +++ b/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 { diff --git a/src/replication/policy/manager.go b/src/replication/policy/manager.go index 6a74d49bd..8f44efa93 100644 --- a/src/replication/policy/manager.go +++ b/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 } diff --git a/src/ui/api/models/replication_policy.go b/src/ui/api/models/replication_policy.go index c8da8a03a..b30e279f9 100644 --- a/src/ui/api/models/replication_policy.go +++ b/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) } } diff --git a/src/ui/api/replication_policy.go b/src/ui/api/replication_policy.go index 93d928eb7..ee6233247 100644 --- a/src/ui/api/replication_policy.go +++ b/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, diff --git a/src/ui/api/replication_policy_test.go b/src/ui/api/replication_policy_test.go index ab3bf3d98..bfbdd32a1 100644 --- a/src/ui/api/replication_policy_test.go +++ b/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) {