From 94138137d50de0fdf719803bac6075c498436cc0 Mon Sep 17 00:00:00 2001 From: Ziming Date: Wed, 28 Aug 2019 16:58:49 +0800 Subject: [PATCH] add valid for rule (#8846) Change-Id: I82215a0cf1ec32a253c8db9bfafe7e25b26c9ad9 Signed-off-by: Ziming Zhang --- src/common/utils/utils.go | 12 +++ src/core/api/retention_test.go | 32 +++--- src/pkg/retention/controller_test.go | 30 ++---- src/pkg/retention/dao/retention_test.go | 8 +- src/pkg/retention/manager_test.go | 26 ++--- src/pkg/retention/policy/models.go | 5 + src/pkg/retention/policy/models_test.go | 102 +++++++++++++++++- .../retention/policy/rule/dayspl/evaluator.go | 23 +++- .../policy/rule/dayspl/evaluator_test.go | 21 ++++ .../retention/policy/rule/daysps/evaluator.go | 23 +++- .../policy/rule/daysps/evaluator_test.go | 21 ++++ src/pkg/retention/policy/rule/evaluator.go | 3 + src/pkg/retention/policy/rule/index/index.go | 54 ++++++++-- .../retention/policy/rule/lastx/evaluator.go | 5 +- .../policy/rule/latestk/evaluator.go | 5 +- .../policy/rule/latestpl/evaluator.go | 24 ++++- .../policy/rule/latestpl/evaluator_test.go | 21 ++++ .../policy/rule/latestps/evaluator.go | 26 ++++- .../policy/rule/latestps/evaluator_test.go | 21 ++++ 19 files changed, 386 insertions(+), 76 deletions(-) diff --git a/src/common/utils/utils.go b/src/common/utils/utils.go index 24a12258d..33206a966 100644 --- a/src/common/utils/utils.go +++ b/src/common/utils/utils.go @@ -267,3 +267,15 @@ func IsContainIllegalChar(s string, illegalChar []string) bool { func IsDigest(ref string) bool { return strings.HasPrefix(ref, "sha256:") && len(ref) == 71 } + +// ParseJSONInt ... +func ParseJSONInt(value interface{}) (int, bool) { + switch value.(type) { + case float64: + return int(value.(float64)), true + case int: + return value.(int), true + default: + return 0, false + } +} diff --git a/src/core/api/retention_test.go b/src/core/api/retention_test.go index 62a86f763..e81632dcf 100644 --- a/src/core/api/retention_test.go +++ b/src/core/api/retention_test.go @@ -36,10 +36,10 @@ func TestCreatePolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -99,10 +99,10 @@ func TestCreatePolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -145,10 +145,10 @@ func TestCreatePolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -170,10 +170,10 @@ func TestCreatePolicy(t *testing.T) { { ID: 2, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -220,10 +220,10 @@ func TestPolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -286,10 +286,10 @@ func TestPolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -334,10 +334,10 @@ func TestPolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -359,10 +359,10 @@ func TestPolicy(t *testing.T) { { ID: 2, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { diff --git a/src/pkg/retention/controller_test.go b/src/pkg/retention/controller_test.go index e8c150d97..692b17407 100644 --- a/src/pkg/retention/controller_test.go +++ b/src/pkg/retention/controller_test.go @@ -39,18 +39,13 @@ func (s *ControllerTestSuite) TestPolicy() { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { - Kind: "label", - Decoration: "with", - Pattern: "latest", - }, - { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: "release-[\\d\\.]+", }, @@ -58,7 +53,7 @@ func (s *ControllerTestSuite) TestPolicy() { ScopeSelectors: map[string][]*rule.Selector{ "repository": { { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: ".+", }, @@ -68,19 +63,14 @@ func (s *ControllerTestSuite) TestPolicy() { { ID: 2, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Disabled: true, Parameters: rule.Parameters{ - "num": 3, + "latestPushedK": 3, }, TagSelectors: []*rule.Selector{ { - Kind: "label", - Decoration: "with", - Pattern: "latest", - }, - { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: "release-[\\d\\.]+", }, @@ -88,7 +78,7 @@ func (s *ControllerTestSuite) TestPolicy() { ScopeSelectors: map[string][]*rule.Selector{ "repository": { { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: ".+", }, @@ -146,9 +136,9 @@ func (s *ControllerTestSuite) TestExecution() { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { diff --git a/src/pkg/retention/dao/retention_test.go b/src/pkg/retention/dao/retention_test.go index 7897dca14..0119d542a 100644 --- a/src/pkg/retention/dao/retention_test.go +++ b/src/pkg/retention/dao/retention_test.go @@ -28,10 +28,10 @@ func TestPolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -102,10 +102,10 @@ func TestExecution(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Action: "retain", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { diff --git a/src/pkg/retention/manager_test.go b/src/pkg/retention/manager_test.go index 83e1ab0ef..690fc2611 100644 --- a/src/pkg/retention/manager_test.go +++ b/src/pkg/retention/manager_test.go @@ -29,18 +29,13 @@ func TestPolicy(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { - Kind: "label", - Decoration: "with", - Pattern: "latest", - }, - { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: "release-[\\d\\.]+", }, @@ -48,7 +43,7 @@ func TestPolicy(t *testing.T) { ScopeSelectors: map[string][]*rule.Selector{ "repository": { { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: ".+", }, @@ -100,18 +95,13 @@ func TestExecution(t *testing.T) { { ID: 1, Priority: 1, - Template: "recentXdays", + Template: "latestPushedK", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { - Kind: "label", - Decoration: "with", - Pattern: "latest", - }, - { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: "release-[\\d\\.]+", }, @@ -119,7 +109,7 @@ func TestExecution(t *testing.T) { ScopeSelectors: map[string][]*rule.Selector{ "repository": { { - Kind: "regularExpression", + Kind: "doublestar", Decoration: "matches", Pattern: ".+", }, diff --git a/src/pkg/retention/policy/models.go b/src/pkg/retention/policy/models.go index f4ab668ea..8cc31e834 100644 --- a/src/pkg/retention/policy/models.go +++ b/src/pkg/retention/policy/models.go @@ -17,6 +17,7 @@ package policy import ( "github.com/astaxie/beego/validation" "github.com/goharbor/harbor/src/pkg/retention/policy/rule" + "github.com/goharbor/harbor/src/pkg/retention/policy/rule/index" ) const ( @@ -78,6 +79,10 @@ func (m *Metadata) Valid(v *validation.Validation) { } if !v.HasErrors() { for _, r := range m.Rules { + if err := index.Valid(r.Template, r.Parameters); err != nil { + _ = v.SetError("Parameters", err.Error()) + return + } if ok, _ := v.Valid(&r); !ok { return } diff --git a/src/pkg/retention/policy/models_test.go b/src/pkg/retention/policy/models_test.go index 07a75b1a4..1ae0ba79c 100644 --- a/src/pkg/retention/policy/models_test.go +++ b/src/pkg/retention/policy/models_test.go @@ -41,9 +41,9 @@ func TestRule(t *testing.T) { ID: 1, Priority: 1, Action: "retain", - Template: "recentXdays", + Template: "latestPushedK", Parameters: rule.Parameters{ - "num": 10, + "latestPushedK": 10, }, TagSelectors: []*rule.Selector{ { @@ -79,7 +79,105 @@ func TestRule(t *testing.T) { require.Nil(t, err) require.False(t, ok) + require.True(t, v.HasErrors()) + require.EqualValues(t, "Kind", v.Errors[0].Field) for _, e := range v.Errors { fmt.Printf("%s %s\n", e.Field, e.Message) } } + +func TestParamValid(t *testing.T) { + p := &Metadata{ + Algorithm: "or", + Rules: []rule.Metadata{ + { + ID: 1, + Priority: 1, + Action: "retain", + Template: "latestPushedK", + Parameters: rule.Parameters{ + "latestPushedK": -10, + }, + TagSelectors: []*rule.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "release-[\\d\\.]+", + }, + }, + ScopeSelectors: map[string][]*rule.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "matches", + Pattern: ".+", + }, + }, + }, + }, + }, + Trigger: &Trigger{ + Kind: "Schedule", + Settings: map[string]interface{}{ + "cron": "* 22 11 * * *", + }, + }, + Scope: &Scope{ + Level: "project", + Reference: 1, + }, + } + v := &validation.Validation{} + ok, err := v.Valid(p) + require.Nil(t, err) + require.False(t, ok) + require.True(t, v.HasErrors()) + require.EqualValues(t, "Parameters", v.Errors[0].Field) + + p = &Metadata{ + Algorithm: "or", + Rules: []rule.Metadata{ + { + ID: 1, + Priority: 1, + Action: "retain", + Template: "nDaysSinceLastPull", + Parameters: rule.Parameters{ + "nDaysSinceLastPull": 20201010, + }, + TagSelectors: []*rule.Selector{ + { + Kind: "doublestar", + Decoration: "matches", + Pattern: "release-[\\d\\.]+", + }, + }, + ScopeSelectors: map[string][]*rule.Selector{ + "repository": { + { + Kind: "doublestar", + Decoration: "matches", + Pattern: ".+", + }, + }, + }, + }, + }, + Trigger: &Trigger{ + Kind: "Schedule", + Settings: map[string]interface{}{ + "cron": "* 22 11 * * *", + }, + }, + Scope: &Scope{ + Level: "project", + Reference: 1, + }, + } + v = &validation.Validation{} + ok, err = v.Valid(p) + require.Nil(t, err) + require.False(t, ok) + require.True(t, v.HasErrors()) + require.EqualValues(t, "Parameters", v.Errors[0].Field) +} diff --git a/src/pkg/retention/policy/rule/dayspl/evaluator.go b/src/pkg/retention/policy/rule/dayspl/evaluator.go index c0fd76256..7face2b73 100644 --- a/src/pkg/retention/policy/rule/dayspl/evaluator.go +++ b/src/pkg/retention/policy/rule/dayspl/evaluator.go @@ -15,6 +15,8 @@ package dayspl import ( + "fmt" + "github.com/goharbor/harbor/src/common/utils" "time" "github.com/goharbor/harbor/src/common/utils/log" @@ -58,7 +60,7 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if p, ok := params[ParameterN]; ok { - if v, ok := p.(float64); ok && v >= 0 { + if v, ok := utils.ParseJSONInt(p); ok && v >= 0 { return &evaluator{n: int(v)} } } @@ -68,3 +70,22 @@ func New(params rule.Parameters) rule.Evaluator { return &evaluator{n: DefaultN} } + +// Valid ... +func Valid(params rule.Parameters) error { + if params != nil { + if p, ok := params[ParameterN]; ok { + if v, ok := utils.ParseJSONInt(p); ok { + if v < 0 { + return fmt.Errorf("%s is less than zero", ParameterN) + } + if v > 20190904 { + return fmt.Errorf("%s is too large", ParameterN) + } + } else { + return fmt.Errorf("%s type error", ParameterN) + } + } + } + return nil +} diff --git a/src/pkg/retention/policy/rule/dayspl/evaluator_test.go b/src/pkg/retention/policy/rule/dayspl/evaluator_test.go index a8587ccd8..49a98d2cb 100644 --- a/src/pkg/retention/policy/rule/dayspl/evaluator_test.go +++ b/src/pkg/retention/policy/rule/dayspl/evaluator_test.go @@ -15,6 +15,7 @@ package dayspl import ( + "errors" "fmt" "testing" "time" @@ -95,6 +96,26 @@ func (e *EvaluatorTestSuite) TestProcess() { } } +func (e *EvaluatorTestSuite) TestValid() { + tests := []struct { + Name string + args rule.Parameters + expectedK error + }{ + {Name: "Valid", args: map[string]rule.Parameter{ParameterN: 5}, expectedK: nil}, + {Name: "Negative", args: map[string]rule.Parameter{ParameterN: -1}, expectedK: errors.New("nDaysSinceLastPull is less than zero")}, + {Name: "Big", args: map[string]rule.Parameter{ParameterN: 21000000}, expectedK: errors.New("nDaysSinceLastPull is too large")}, + } + + for _, tt := range tests { + e.T().Run(tt.Name, func(t *testing.T) { + err := Valid(tt.args) + + require.Equal(t, tt.expectedK, err) + }) + } +} + func TestEvaluatorSuite(t *testing.T) { suite.Run(t, &EvaluatorTestSuite{}) } diff --git a/src/pkg/retention/policy/rule/daysps/evaluator.go b/src/pkg/retention/policy/rule/daysps/evaluator.go index ee4dd436d..58cf73d2b 100644 --- a/src/pkg/retention/policy/rule/daysps/evaluator.go +++ b/src/pkg/retention/policy/rule/daysps/evaluator.go @@ -15,6 +15,8 @@ package daysps import ( + "fmt" + "github.com/goharbor/harbor/src/common/utils" "time" "github.com/goharbor/harbor/src/common/utils/log" @@ -58,7 +60,7 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if p, ok := params[ParameterN]; ok { - if v, ok := p.(float64); ok && v >= 0 { + if v, ok := utils.ParseJSONInt(p); ok && v >= 0 { return &evaluator{n: int(v)} } } @@ -68,3 +70,22 @@ func New(params rule.Parameters) rule.Evaluator { return &evaluator{n: DefaultN} } + +// Valid ... +func Valid(params rule.Parameters) error { + if params != nil { + if p, ok := params[ParameterN]; ok { + if v, ok := utils.ParseJSONInt(p); ok { + if v < 0 { + return fmt.Errorf("%s is less than zero", ParameterN) + } + if v > 20190904 { + return fmt.Errorf("%s is too large", ParameterN) + } + } else { + return fmt.Errorf("%s type error", ParameterN) + } + } + } + return nil +} diff --git a/src/pkg/retention/policy/rule/daysps/evaluator_test.go b/src/pkg/retention/policy/rule/daysps/evaluator_test.go index 75287ce4f..a40c2c5a2 100644 --- a/src/pkg/retention/policy/rule/daysps/evaluator_test.go +++ b/src/pkg/retention/policy/rule/daysps/evaluator_test.go @@ -15,6 +15,7 @@ package daysps import ( + "errors" "fmt" "testing" "time" @@ -95,6 +96,26 @@ func (e *EvaluatorTestSuite) TestProcess() { } } +func (e *EvaluatorTestSuite) TestValid() { + tests := []struct { + Name string + args rule.Parameters + expectedK error + }{ + {Name: "Valid", args: map[string]rule.Parameter{ParameterN: 5}, expectedK: nil}, + {Name: "Negative", args: map[string]rule.Parameter{ParameterN: -1}, expectedK: errors.New("nDaysSinceLastPush is less than zero")}, + {Name: "Big", args: map[string]rule.Parameter{ParameterN: 21000000}, expectedK: errors.New("nDaysSinceLastPush is too large")}, + } + + for _, tt := range tests { + e.T().Run(tt.Name, func(t *testing.T) { + err := Valid(tt.args) + + require.Equal(t, tt.expectedK, err) + }) + } +} + func TestEvaluatorSuite(t *testing.T) { suite.Run(t, &EvaluatorTestSuite{}) } diff --git a/src/pkg/retention/policy/rule/evaluator.go b/src/pkg/retention/policy/rule/evaluator.go index 91ec27913..18e641986 100644 --- a/src/pkg/retention/policy/rule/evaluator.go +++ b/src/pkg/retention/policy/rule/evaluator.go @@ -34,3 +34,6 @@ type Evaluator interface { // Factory defines a factory method for creating rule evaluator type Factory func(parameters Parameters) Evaluator + +// Validator ... +type Validator func(parameters Parameters) error diff --git a/src/pkg/retention/policy/rule/index/index.go b/src/pkg/retention/policy/rule/index/index.go index 7360a2cb8..45b1922b2 100644 --- a/src/pkg/retention/policy/rule/index/index.go +++ b/src/pkg/retention/policy/rule/index/index.go @@ -61,6 +61,8 @@ type indexedItem struct { Meta *Metadata Factory rule.Factory + + Validators []rule.Validator } func init() { @@ -76,7 +78,7 @@ func init() { Required: true, }, }, - }, latestps.New) + }, latestps.New, latestps.Valid) // Register latest pulled Register(&Metadata{ @@ -90,7 +92,7 @@ func init() { Required: true, }, }, - }, latestpl.New) + }, latestpl.New, latestpl.Valid) // Register latest active Register(&Metadata{ @@ -146,7 +148,7 @@ func init() { Required: true, }, }, - }, dayspl.New) + }, dayspl.New, dayspl.Valid) // Register daysps Register(&Metadata{ @@ -160,22 +162,60 @@ func init() { Required: true, }, }, - }, daysps.New) + }, daysps.New, daysps.Valid) } // Register the rule evaluator with the corresponding rule template -func Register(meta *Metadata, factory rule.Factory) { +func Register(meta *Metadata, factory rule.Factory, validator ...rule.Validator) { if meta == nil || factory == nil || len(meta.TemplateID) == 0 { // do nothing return } index.Store(meta.TemplateID, &indexedItem{ - Meta: meta, - Factory: factory, + Meta: meta, + Factory: factory, + Validators: validator, }) } +// Valid ... +func Valid(templateID string, parameters rule.Parameters) error { + if len(templateID) == 0 { + return errors.New("empty rule template ID") + } + + v, ok := index.Load(templateID) + if !ok { + return errors.Errorf("rule evaluator %s is not registered", templateID) + } + + item := v.(*indexedItem) + + // We can check more things if we want to do in the future + if len(item.Meta.Parameters) > 0 { + for _, p := range item.Meta.Parameters { + if p.Required { + exists := parameters != nil + if exists { + _, exists = parameters[p.Name] + } + + if !exists { + return errors.Errorf("missing required parameter %s for rule %s", p.Name, templateID) + } + } + } + } + for _, v := range item.Validators { + err := v(parameters) + if err != nil { + return err + } + } + return nil +} + // Get rule evaluator with the provided template ID func Get(templateID string, parameters rule.Parameters) (rule.Evaluator, error) { if len(templateID) == 0 { diff --git a/src/pkg/retention/policy/rule/lastx/evaluator.go b/src/pkg/retention/policy/rule/lastx/evaluator.go index b466f5eda..7457c0db3 100644 --- a/src/pkg/retention/policy/rule/lastx/evaluator.go +++ b/src/pkg/retention/policy/rule/lastx/evaluator.go @@ -15,6 +15,7 @@ package lastx import ( + "github.com/goharbor/harbor/src/common/utils" "time" "github.com/goharbor/harbor/src/common/utils/log" @@ -58,8 +59,8 @@ func (e *evaluator) Action() string { // New a Evaluator func New(params rule.Parameters) rule.Evaluator { if params != nil { - if param, ok := params[ParameterX]; ok { - if v, ok := param.(float64); ok && v >= 0 { + if p, ok := params[ParameterX]; ok { + if v, ok := utils.ParseJSONInt(p); ok && v >= 0 { return &evaluator{ x: int(v), } diff --git a/src/pkg/retention/policy/rule/latestk/evaluator.go b/src/pkg/retention/policy/rule/latestk/evaluator.go index f6d73599a..9f9610f55 100644 --- a/src/pkg/retention/policy/rule/latestk/evaluator.go +++ b/src/pkg/retention/policy/rule/latestk/evaluator.go @@ -15,6 +15,7 @@ package latestk import ( + "github.com/goharbor/harbor/src/common/utils" "sort" "github.com/goharbor/harbor/src/common/utils/log" @@ -64,8 +65,8 @@ func (e *evaluator) Action() string { // New a Evaluator func New(params rule.Parameters) rule.Evaluator { if params != nil { - if param, ok := params[ParameterK]; ok { - if v, ok := param.(float64); ok && v >= 0 { + if p, ok := params[ParameterK]; ok { + if v, ok := utils.ParseJSONInt(p); ok && v >= 0 { return &evaluator{ k: int(v), } diff --git a/src/pkg/retention/policy/rule/latestpl/evaluator.go b/src/pkg/retention/policy/rule/latestpl/evaluator.go index bed7b6e4e..21381759c 100644 --- a/src/pkg/retention/policy/rule/latestpl/evaluator.go +++ b/src/pkg/retention/policy/rule/latestpl/evaluator.go @@ -15,6 +15,9 @@ package latestpl import ( + "fmt" + "github.com/goharbor/harbor/src/common/utils" + "math" "sort" "github.com/goharbor/harbor/src/common/utils/log" @@ -59,7 +62,7 @@ func (e *evaluator) Action() string { func New(params rule.Parameters) rule.Evaluator { if params != nil { if p, ok := params[ParameterN]; ok { - if v, ok := p.(float64); ok && v >= 0 { + if v, ok := utils.ParseJSONInt(p); ok && v >= 0 { return &evaluator{n: int(v)} } } @@ -69,3 +72,22 @@ func New(params rule.Parameters) rule.Evaluator { return &evaluator{n: DefaultN} } + +// Valid ... +func Valid(params rule.Parameters) error { + if params != nil { + if p, ok := params[ParameterN]; ok { + if v, ok := utils.ParseJSONInt(p); ok { + if v < 0 { + return fmt.Errorf("%s is less than zero", ParameterN) + } + if v >= math.MaxInt16 { + return fmt.Errorf("%s is too large", ParameterN) + } + } else { + return fmt.Errorf("%s type error", ParameterN) + } + } + } + return nil +} diff --git a/src/pkg/retention/policy/rule/latestpl/evaluator_test.go b/src/pkg/retention/policy/rule/latestpl/evaluator_test.go index 69b0605f5..ebd1679ae 100644 --- a/src/pkg/retention/policy/rule/latestpl/evaluator_test.go +++ b/src/pkg/retention/policy/rule/latestpl/evaluator_test.go @@ -15,6 +15,7 @@ package latestpl import ( + "errors" "fmt" "math/rand" "testing" @@ -84,6 +85,26 @@ func (e *EvaluatorTestSuite) TestProcess() { } } +func (e *EvaluatorTestSuite) TestValid() { + tests := []struct { + Name string + args rule.Parameters + expectedK error + }{ + {Name: "Valid", args: map[string]rule.Parameter{ParameterN: 5}, expectedK: nil}, + {Name: "Negative", args: map[string]rule.Parameter{ParameterN: -1}, expectedK: errors.New("latestPulledN is less than zero")}, + {Name: "Big", args: map[string]rule.Parameter{ParameterN: 40000}, expectedK: errors.New("latestPulledN is too large")}, + } + + for _, tt := range tests { + e.T().Run(tt.Name, func(t *testing.T) { + err := Valid(tt.args) + + require.Equal(t, tt.expectedK, err) + }) + } +} + func TestEvaluatorSuite(t *testing.T) { suite.Run(t, &EvaluatorTestSuite{}) } diff --git a/src/pkg/retention/policy/rule/latestps/evaluator.go b/src/pkg/retention/policy/rule/latestps/evaluator.go index 21d1b77f7..f672aa1c6 100644 --- a/src/pkg/retention/policy/rule/latestps/evaluator.go +++ b/src/pkg/retention/policy/rule/latestps/evaluator.go @@ -15,6 +15,9 @@ package latestps import ( + "fmt" + "github.com/goharbor/harbor/src/common/utils" + "math" "sort" "github.com/goharbor/harbor/src/common/utils/log" @@ -61,8 +64,8 @@ func (e *evaluator) Action() string { // New a Evaluator func New(params rule.Parameters) rule.Evaluator { if params != nil { - if param, ok := params[ParameterK]; ok { - if v, ok := param.(float64); ok && v >= 0 { + if p, ok := params[ParameterK]; ok { + if v, ok := utils.ParseJSONInt(p); ok && v >= 0 { return &evaluator{ k: int(v), } @@ -76,3 +79,22 @@ func New(params rule.Parameters) rule.Evaluator { k: DefaultK, } } + +// Valid ... +func Valid(params rule.Parameters) error { + if params != nil { + if p, ok := params[ParameterK]; ok { + if v, ok := utils.ParseJSONInt(p); ok { + if v < 0 { + return fmt.Errorf("%s is less than zero", ParameterK) + } + if v >= math.MaxInt16 { + return fmt.Errorf("%s is too large", ParameterK) + } + } else { + return fmt.Errorf("%s type error", ParameterK) + } + } + } + return nil +} diff --git a/src/pkg/retention/policy/rule/latestps/evaluator_test.go b/src/pkg/retention/policy/rule/latestps/evaluator_test.go index 6e303c3c4..38fa64570 100644 --- a/src/pkg/retention/policy/rule/latestps/evaluator_test.go +++ b/src/pkg/retention/policy/rule/latestps/evaluator_test.go @@ -1,6 +1,7 @@ package latestps import ( + "errors" "fmt" "math/rand" "testing" @@ -66,6 +67,26 @@ func (e *EvaluatorTestSuite) TestProcess() { } } +func (e *EvaluatorTestSuite) TestValid() { + tests := []struct { + Name string + args rule.Parameters + expectedK error + }{ + {Name: "Valid", args: map[string]rule.Parameter{ParameterK: 5}, expectedK: nil}, + {Name: "Negative", args: map[string]rule.Parameter{ParameterK: -1}, expectedK: errors.New("latestPushedK is less than zero")}, + {Name: "Big", args: map[string]rule.Parameter{ParameterK: 40000}, expectedK: errors.New("latestPushedK is too large")}, + } + + for _, tt := range tests { + e.T().Run(tt.Name, func(t *testing.T) { + err := Valid(tt.args) + + require.Equal(t, tt.expectedK, err) + }) + } +} + func TestEvaluator(t *testing.T) { suite.Run(t, &EvaluatorTestSuite{}) }