From 2d3a354bfad5431888db896dc0441380ae6abb2d Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 22 Mar 2019 17:03:38 +0100 Subject: [PATCH 01/48] Introduce the RulesetVersion type --- ruleset.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ruleset.go b/ruleset.go index eae216a..e2bf37e 100644 --- a/ruleset.go +++ b/ruleset.go @@ -102,3 +102,9 @@ func (s *Signature) Validate() error { return nil } + +// A RulesetVersion describes a version of a list of rules. +type RulesetVersion struct { + Version string + Rules []*rule.Rule +} From 91d3a055fbeb7a1084d1ceb1facb82cdda95c71a Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 22 Mar 2019 17:34:17 +0100 Subject: [PATCH 02/48] NewRuleset creates one version --- ruleset.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ruleset.go b/ruleset.go index e2bf37e..6749408 100644 --- a/ruleset.go +++ b/ruleset.go @@ -8,18 +8,19 @@ import ( // A Ruleset is a list of rules and their metadata. type Ruleset struct { - Path string `json:"path"` - Version string `json:"version,omitempty"` - Rules []*rule.Rule `json:"rules,omitempty"` - Signature *Signature `json:"signature,omitempty"` - Versions []string `json:"versions,omitempty"` + Path string `json:"path"` + Signature *Signature `json:"signature,omitempty"` + Versions []RulesetVersion `json:"versions,omitempty"` } // NewRuleset creates a ruleset. func NewRuleset(rules ...*rule.Rule) *Ruleset { - rs := Ruleset{ - Rules: rules, - } + var rs Ruleset + + rs.Versions = append(rs.Versions, RulesetVersion{ + Version: "latest", + Rules: rules, + }) return &rs } From 11c0bbe5fc44eb2555489d6a2801470e373421c7 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 22 Mar 2019 17:35:49 +0100 Subject: [PATCH 03/48] Ruleset Eval method evaluates latest ruleset --- ruleset.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ruleset.go b/ruleset.go index 6749408..06fff06 100644 --- a/ruleset.go +++ b/ruleset.go @@ -28,7 +28,11 @@ func NewRuleset(rules ...*rule.Rule) *Ruleset { // Eval evaluates every rule of the ruleset until one matches. // It returns rule.ErrNoMatch if no rule matches the given context. func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { - for _, rl := range r.Rules { + if len(r.Versions) == 0 { + return nil, rerrors.ErrNoMatch + } + + for _, rl := range r.Versions[len(r.Versions)-1].Rules { res, err := rl.Eval(params) if err != rerrors.ErrNoMatch { return res, err From b91bb05c5b5bb3b27cd0f165fb86514dbafde78c Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 22 Mar 2019 17:41:37 +0100 Subject: [PATCH 04/48] Remove unused methods --- ruleset.go | 43 +------------------------------------------ ruleset_test.go | 13 ------------- 2 files changed, 1 insertion(+), 55 deletions(-) diff --git a/ruleset.go b/ruleset.go index 06fff06..df5ea18 100644 --- a/ruleset.go +++ b/ruleset.go @@ -25,7 +25,7 @@ func NewRuleset(rules ...*rule.Rule) *Ruleset { return &rs } -// Eval evaluates every rule of the ruleset until one matches. +// Eval evaluates the latest ruleset version, evaluating every rule until one matches. // It returns rule.ErrNoMatch if no rule matches the given context. func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { if len(r.Versions) == 0 { @@ -42,47 +42,6 @@ func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { return nil, rerrors.ErrNoMatch } -// Params returns a list of all the parameters used in all the underlying rules. -func (r *Ruleset) Params() []rule.Param { - bm := make(map[string]bool) - var params []rule.Param - - for _, rl := range r.Rules { - ps := rl.Params() - for _, p := range ps { - if !bm[p.Name] { - params = append(params, p) - bm[p.Name] = true - } - } - } - - return params -} - -// ValidateSignature validates the ruleset against the given signature. -func (r *Ruleset) ValidateSignature(signature *Signature) error { - if err := signature.Validate(); err != nil { - return err - } - - for _, rl := range r.Rules { - if rl.Result.Type != signature.ReturnType { - return rerrors.ErrSignatureMismatch - } - - ps := rl.Params() - for _, p := range ps { - tp, ok := signature.Params[p.Name] - if !ok || p.Type != tp { - return rerrors.ErrSignatureMismatch - } - } - } - - return nil -} - // Signature represents the signature of a ruleset. type Signature struct { ReturnType string `json:"returnType"` diff --git a/ruleset_test.go b/ruleset_test.go index 4ade24b..5462cb3 100644 --- a/ruleset_test.go +++ b/ruleset_test.go @@ -55,16 +55,3 @@ func TestRulesetEval(t *testing.T) { require.Equal(t, "default", res.Data) }) } - -func TestRulesetParams(t *testing.T) { - r1 := NewRuleset( - rule.New(rule.Eq(rule.StringParam("foo"), rule.Int64Param("bar")), rule.StringValue("first")), - rule.New(rule.Eq(rule.StringParam("foo"), rule.Float64Param("baz")), rule.StringValue("second")), - rule.New(rule.True(), rule.StringValue("default")), - ) - require.Equal(t, []rule.Param{ - *rule.StringParam("foo"), - *rule.Int64Param("bar"), - *rule.Float64Param("baz"), - }, r1.Params()) -} From ce9dbbef9ea584f63cdde272f50e9d29d5872192 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 27 Mar 2019 16:26:24 +0100 Subject: [PATCH 05/48] Remove ErrTypeMismatch --- engine.go | 2 +- errors/errors.go | 6 ++--- example_test.go | 58 +++++++++++++++++++++++++++++++++++------------- 3 files changed, 46 insertions(+), 20 deletions(-) diff --git a/engine.go b/engine.go index 84e682b..e2e1fd4 100644 --- a/engine.go +++ b/engine.go @@ -42,7 +42,7 @@ func (e *Engine) get(ctx context.Context, typ, path string, params rule.Params, } if typ != "" && result.Value.Type != typ { - return nil, rerrors.ErrTypeMismatch + return nil, errors.New("type returned by rule doesn't match") } return result, nil diff --git a/errors/errors.go b/errors/errors.go index 17f5394..4354cfc 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -3,12 +3,12 @@ package errors import "errors" var ( - // ErrTypeMismatch is returned when the evaluated rule doesn't return the expected result type. - ErrTypeMismatch = errors.New("type returned by rule doesn't match") - // ErrRulesetNotFound must be returned when no ruleset is found for a given key. ErrRulesetNotFound = errors.New("ruleset not found") + // ErrRulesetVersionNotFound must be returned when a version of a ruleset is not found for a given key. + ErrRulesetVersionNotFound = errors.New("ruleset version not found") + // ErrParamTypeMismatch is returned when a parameter type is different from expected. ErrParamTypeMismatch = errors.New("parameter type mismatches") diff --git a/example_test.go b/example_test.go index c65ce0a..91f5e5f 100644 --- a/example_test.go +++ b/example_test.go @@ -55,38 +55,68 @@ func init() { ev = buf buf.Add("/a/b/c", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("some-string")), + Versions: []regula.RulesetVersion{ + { + Version: "latest", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("some-string")), + }, + }, }, }) buf.Add("/path/to/string/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("some-string")), + Versions: []regula.RulesetVersion{ + { + Version: "latest", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("some-string")), + }, + }, }, }) buf.Add("/path/to/int64/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.Int64Value(10)), + Versions: []regula.RulesetVersion{ + { + Version: "latest", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.Int64Value(10)), + }, + }, }, }) buf.Add("/path/to/float64/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.Float64Value(3.14)), + Versions: []regula.RulesetVersion{ + { + Version: "latest", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.Float64Value(3.14)), + }, + }, }, }) buf.Add("/path/to/bool/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.BoolValue(true)), + Versions: []regula.RulesetVersion{ + { + Version: "latest", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.BoolValue(true)), + }, + }, }, }) buf.Add("/path/to/duration/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("3s")), + Versions: []regula.RulesetVersion{ + { + Version: "latest", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("3s")), + }, + }, }, }) } @@ -101,10 +131,6 @@ func ExampleEngine() { if err != nil { switch err { - case errors.ErrRulesetNotFound: - // when the ruleset doesn't exist - case errors.ErrTypeMismatch: - // when the ruleset returns the bad type case errors.ErrNoMatch: // when the ruleset doesn't match default: From ddbf01228cbe13dabc80a72ec7c19d730008ee36 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 27 Mar 2019 16:38:31 +0100 Subject: [PATCH 06/48] Define constant sentinel errors --- api/service.go | 14 +++++++------- engine_test.go | 4 ++-- errors/errors.go | 21 +++++++++++++-------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/api/service.go b/api/service.go index f71198c..fe8937d 100644 --- a/api/service.go +++ b/api/service.go @@ -3,19 +3,19 @@ package api import ( "context" - "errors" "github.com/heetch/regula" + "github.com/heetch/regula/errors" "github.com/heetch/regula/rule" ) // API errors. -var ( - ErrRulesetNotFound = errors.New("ruleset not found") - ErrRulesetNotModified = errors.New("not modified") - ErrSignatureNotFound = errors.New("signature not found") - ErrInvalidContinueToken = errors.New("invalid continue token") - ErrAlreadyExists = errors.New("already exists") +const ( + ErrRulesetNotFound = errors.Error("ruleset not found") + ErrRulesetNotModified = errors.Error("not modified") + ErrSignatureNotFound = errors.Error("signature not found") + ErrInvalidContinueToken = errors.Error("invalid continue token") + ErrAlreadyExists = errors.Error("already exists") ) // RulesetService is a service managing rulesets. diff --git a/engine_test.go b/engine_test.go index 080fcf1..de7bcf0 100644 --- a/engine_test.go +++ b/engine_test.go @@ -96,10 +96,10 @@ func TestEngine(t *testing.T) { require.Equal(t, -3.14, f) _, err = e.GetString(ctx, "match-bool", nil) - require.Equal(t, errors.ErrTypeMismatch, err) + require.Error(t, err) _, err = e.GetString(ctx, "type-mismatch", nil) - require.Equal(t, errors.ErrTypeMismatch, err) + require.Error(t, err) _, err = e.GetString(ctx, "no-match", nil) require.Equal(t, errors.ErrNoMatch, err) diff --git a/errors/errors.go b/errors/errors.go index 4354cfc..8dcb455 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -1,23 +1,28 @@ package errors -import "errors" +// Error is used to define constant, comparable errors. +type Error string -var ( +func (e Error) Error() string { + return string(e) +} + +const ( // ErrRulesetNotFound must be returned when no ruleset is found for a given key. - ErrRulesetNotFound = errors.New("ruleset not found") + ErrRulesetNotFound = Error("ruleset not found") // ErrRulesetVersionNotFound must be returned when a version of a ruleset is not found for a given key. - ErrRulesetVersionNotFound = errors.New("ruleset version not found") + ErrRulesetVersionNotFound = Error("ruleset version not found") // ErrParamTypeMismatch is returned when a parameter type is different from expected. - ErrParamTypeMismatch = errors.New("parameter type mismatches") + ErrParamTypeMismatch = Error("parameter type mismatches") // ErrSignatureMismatch is returned when a rule doesn't respect a given signature. - ErrSignatureMismatch = errors.New("signature mismatch") + ErrSignatureMismatch = Error("signature mismatch") // ErrParamNotFound is returned when a parameter is not defined. - ErrParamNotFound = errors.New("parameter not found") + ErrParamNotFound = Error("parameter not found") // ErrNoMatch is returned when the rule doesn't match the given params. - ErrNoMatch = errors.New("rule doesn't match the given params") + ErrNoMatch = Error("rule doesn't match the given params") ) From ad44c43714df35250752cd0e2347b97e892d93c1 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 27 Mar 2019 16:51:40 +0100 Subject: [PATCH 07/48] Add EvalVersion to Ruleset type --- ruleset.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/ruleset.go b/ruleset.go index df5ea18..32ef8d4 100644 --- a/ruleset.go +++ b/ruleset.go @@ -42,6 +42,27 @@ func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { return nil, rerrors.ErrNoMatch } +// EvalVersion evaluates a version of the ruleset, evaluating every rule until one matches. +// It returns rule.ErrNoMatch if no rule matches the given context. +func (r *Ruleset) EvalVersion(version string, params rule.Params) (*rule.Value, error) { + if len(r.Versions) == 0 { + return nil, rerrors.ErrNoMatch + } + + for _, rv := range r.Versions { + if rv.Version == version { + for _, rl := range rv.Rules { + res, err := rl.Eval(params) + if err != rerrors.ErrNoMatch { + return res, err + } + } + } + } + + return nil, rerrors.ErrNoMatch +} + // Signature represents the signature of a ruleset. type Signature struct { ReturnType string `json:"returnType"` From 62d9ad60575fc000989eaa8baca52e0237ad0e9d Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 27 Mar 2019 16:59:06 +0100 Subject: [PATCH 08/48] Simplify RulesetBuffer --- engine.go | 83 ++++++++++++++++-------------------------------------- ruleset.go | 2 +- 2 files changed, 25 insertions(+), 60 deletions(-) diff --git a/engine.go b/engine.go index e2e1fd4..8626c40 100644 --- a/engine.go +++ b/engine.go @@ -150,53 +150,34 @@ type EvalResult struct { // It is safe for concurrent use. type RulesetBuffer struct { rw sync.RWMutex - rulesets map[string][]*rulesetInfo + rulesets map[string]*Ruleset } // NewRulesetBuffer creates a ready to use RulesetBuffer. func NewRulesetBuffer() *RulesetBuffer { return &RulesetBuffer{ - rulesets: make(map[string][]*rulesetInfo), + rulesets: make(map[string]*Ruleset), } } -type rulesetInfo struct { - path, version string - r *Ruleset -} - -// Add adds the given ruleset version to a list for a specific path. -// The last added ruleset is treated as the latest version. -func (b *RulesetBuffer) Add(path, version string, r *Ruleset) { +// Set the given ruleset to a list for a specific path. +func (b *RulesetBuffer) Set(path string, r *Ruleset) { b.rw.Lock() - b.rulesets[path] = append(b.rulesets[path], &rulesetInfo{path, version, r}) + b.rulesets[path] = r b.rw.Unlock() } -// Latest returns the latest version of a ruleset. -func (b *RulesetBuffer) Latest(path string) (*Ruleset, string, error) { +// Get a ruleset associated with path. +func (b *RulesetBuffer) Get(path string) (*Ruleset, error) { b.rw.RLock() defer b.rw.RUnlock() - l, ok := b.rulesets[path] - if !ok || len(l) == 0 { - return nil, "", rerrors.ErrRulesetNotFound + r, ok := b.rulesets[path] + if !ok { + return nil, errors.New("ruleset not found") } - return l[len(l)-1].r, l[len(l)-1].version, nil -} - -// GetVersion returns a ruleset associated with the given path and version. -func (b *RulesetBuffer) GetVersion(path, version string) (*Ruleset, error) { - b.rw.RLock() - defer b.rw.RUnlock() - - ri, err := b.getVersion(path, version) - if err != nil { - return nil, err - } - - return ri.r, nil + return r, nil } // Eval evaluates the selected ruleset version, or the latest one if the version is empty, and returns errors.ErrRulesetNotFound if not found. @@ -204,45 +185,29 @@ func (b *RulesetBuffer) Eval(ctx context.Context, path, version string, params r b.rw.RLock() defer b.rw.RUnlock() - var ri *rulesetInfo - var err error + r, ok := b.rulesets[path] + if !ok || len(r.Versions) == 0 { + return nil, rerrors.ErrRulesetVersionNotFound + } - if version == "" { - l, ok := b.rulesets[path] - if !ok || len(l) == 0 { - return nil, rerrors.ErrRulesetNotFound - } + var ( + v *rule.Value + err error + ) - ri = l[len(l)-1] + if version == "" { + version = r.Versions[len(r.Versions)-1].Version + v, err = r.Eval(params) } else { - ri, err = b.getVersion(path, version) - if err != nil { - return nil, err - } + v, err = r.EvalVersion(version, params) } - v, err := ri.r.Eval(params) if err != nil { return nil, err } return &EvalResult{ Value: v, - Version: ri.version, + Version: version, }, nil } - -func (b *RulesetBuffer) getVersion(path, version string) (*rulesetInfo, error) { - l, ok := b.rulesets[path] - if !ok || len(l) == 0 { - return nil, rerrors.ErrRulesetNotFound - } - - for _, ri := range l { - if ri.version == version { - return ri, nil - } - } - - return nil, rerrors.ErrRulesetNotFound -} diff --git a/ruleset.go b/ruleset.go index 32ef8d4..97f87b6 100644 --- a/ruleset.go +++ b/ruleset.go @@ -39,7 +39,7 @@ func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { } } - return nil, rerrors.ErrNoMatch + return nil, rerrors.ErrRulesetVersionNotFound } // EvalVersion evaluates a version of the ruleset, evaluating every rule until one matches. From 740a1d6df2ef1db57ff9c3775538f0a583391cd9 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 27 Mar 2019 17:03:47 +0100 Subject: [PATCH 09/48] Fix engine and example tests --- engine_test.go | 108 ++++++++++++++++++++++++++++++++---------------- example_test.go | 24 +++++------ 2 files changed, 84 insertions(+), 48 deletions(-) diff --git a/engine_test.go b/engine_test.go index de7bcf0..960629d 100644 --- a/engine_test.go +++ b/engine_test.go @@ -16,50 +16,86 @@ func TestEngine(t *testing.T) { buf := regula.NewRulesetBuffer() - buf.Add("match-string-a", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v1")), + buf.Set("match-string-a", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v1")), + }, + }, + { + Version: "2", + Rules: []*rule.Rule{ + rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v2")), + }, + }, }, }) - buf.Add("match-string-a", "2", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v2")), - }, + + buf.Set("match-string-b", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("matched b")), + }, + }}, }) - buf.Add("match-string-b", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("matched b")), - }, + + buf.Set("type-mismatch", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "int", Data: "5"}), + }, + }}, }) - buf.Add("type-mismatch", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "int", Data: "5"}), - }, + buf.Set("no-match", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("bar")), rule.StringValue("matched d")), + }, + }}, }) - buf.Add("no-match", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("bar")), rule.StringValue("matched d")), - }, + buf.Set("match-bool", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "bool", Data: "true"}), + }, + }}, }) - buf.Add("match-bool", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "bool", Data: "true"}), - }, + buf.Set("match-int64", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "int64", Data: "-10"}), + }, + }}, }) - buf.Add("match-int64", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "int64", Data: "-10"}), - }, + buf.Set("match-float64", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "float64", Data: "-3.14"}), + }, + }}, }) - buf.Add("match-float64", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "float64", Data: "-3.14"}), - }, - }) - buf.Add("match-duration", "1", ®ula.Ruleset{ - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("3s")), - }, + buf.Set("match-duration", ®ula.Ruleset{ + Versions: []regula.RulesetVersion{ + { + Version: "1", + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("3s")), + }, + }}, }) e := regula.NewEngine(buf) diff --git a/example_test.go b/example_test.go index 91f5e5f..d391516 100644 --- a/example_test.go +++ b/example_test.go @@ -54,10 +54,10 @@ func init() { buf := regula.NewRulesetBuffer() ev = buf - buf.Add("/a/b/c", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + buf.Set("/a/b/c", ®ula.Ruleset{ Versions: []regula.RulesetVersion{ { - Version: "latest", + Version: "5b4cbdf307bb5346a6c42ac3", Rules: []*rule.Rule{ rule.New(rule.True(), rule.StringValue("some-string")), }, @@ -65,10 +65,10 @@ func init() { }, }) - buf.Add("/path/to/string/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + buf.Set("/path/to/string/key", ®ula.Ruleset{ Versions: []regula.RulesetVersion{ { - Version: "latest", + Version: "5b4cbdf307bb5346a6c42ac3", Rules: []*rule.Rule{ rule.New(rule.True(), rule.StringValue("some-string")), }, @@ -76,10 +76,10 @@ func init() { }, }) - buf.Add("/path/to/int64/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + buf.Set("/path/to/int64/key", ®ula.Ruleset{ Versions: []regula.RulesetVersion{ { - Version: "latest", + Version: "5b4cbdf307bb5346a6c42ac3", Rules: []*rule.Rule{ rule.New(rule.True(), rule.Int64Value(10)), }, @@ -87,10 +87,10 @@ func init() { }, }) - buf.Add("/path/to/float64/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + buf.Set("/path/to/float64/key", ®ula.Ruleset{ Versions: []regula.RulesetVersion{ { - Version: "latest", + Version: "5b4cbdf307bb5346a6c42ac3", Rules: []*rule.Rule{ rule.New(rule.True(), rule.Float64Value(3.14)), }, @@ -98,10 +98,10 @@ func init() { }, }) - buf.Add("/path/to/bool/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + buf.Set("/path/to/bool/key", ®ula.Ruleset{ Versions: []regula.RulesetVersion{ { - Version: "latest", + Version: "5b4cbdf307bb5346a6c42ac3", Rules: []*rule.Rule{ rule.New(rule.True(), rule.BoolValue(true)), }, @@ -109,10 +109,10 @@ func init() { }, }) - buf.Add("/path/to/duration/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + buf.Set("/path/to/duration/key", ®ula.Ruleset{ Versions: []regula.RulesetVersion{ { - Version: "latest", + Version: "5b4cbdf307bb5346a6c42ac3", Rules: []*rule.Rule{ rule.New(rule.True(), rule.StringValue("3s")), }, From b50370241e4fe8c0752c47acd15e52b21653dcb5 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 27 Mar 2019 17:04:58 +0100 Subject: [PATCH 10/48] Remove unused ErrSignatureMismatch --- errors/errors.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 8dcb455..7cc6cfd 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -17,9 +17,6 @@ const ( // ErrParamTypeMismatch is returned when a parameter type is different from expected. ErrParamTypeMismatch = Error("parameter type mismatches") - // ErrSignatureMismatch is returned when a rule doesn't respect a given signature. - ErrSignatureMismatch = Error("signature mismatch") - // ErrParamNotFound is returned when a parameter is not defined. ErrParamNotFound = Error("parameter not found") From 70278fa4c56df603217403fd9aa6951c9220ace7 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 11:12:48 +0100 Subject: [PATCH 11/48] Adapt API Get to the new Ruleset format --- api/etcd/get.go | 43 ++++++++++++++++--------------------------- api/service.go | 5 ++--- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/api/etcd/get.go b/api/etcd/get.go index a5102ca..0dbd7c3 100644 --- a/api/etcd/get.go +++ b/api/etcd/get.go @@ -11,30 +11,24 @@ import ( "github.com/pkg/errors" ) -// Get returns the ruleset related to the given path. By default, it returns the latest one. -// It returns the related ruleset version if it's specified. -func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula.Ruleset, error) { +// Get returns the ruleset related to the given path. +func (s *RulesetService) Get(ctx context.Context, path string) (*regula.Ruleset, error) { if path == "" { return nil, api.ErrRulesetNotFound } ops := []clientv3.Op{ clientv3.OpGet(s.signaturesPath(path)), - clientv3.OpGet(s.versionsPath(path)), + clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithLimit(100), clientv3.WithSort(clientv3.SortByKey, clientv3.SortDescend)), } - if version == "" { - ops = append(ops, clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithLastKey()...)) - } else { - ops = append(ops, clientv3.OpGet(s.rulesPath(path, version))) - } // running all the requests within a single transaction so only one network round trip is performed. resp, err := s.Client.KV.Txn(ctx).Then(ops...).Commit() if err != nil { return nil, errors.Wrapf(err, "failed to fetch ruleset: %s", path) } - if len(resp.Responses) != 3 { + if len(resp.Responses) != 2 { return nil, api.ErrRulesetNotFound } @@ -49,28 +43,23 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula return nil, errors.Wrap(err, "failed to unmarshal signature") } - // decode versions - var versions pb.Versions - err = proto.Unmarshal(resp.Responses[1].GetResponseRange().Kvs[0].Value, &versions) - if err != nil { - return nil, errors.Wrap(err, "failed to unmarshal versions") - } + // decode versions of rules. they are returned by etcd ordered by key descendend. + // this loop rearranges them in ascending order. + versions := make([]regula.RulesetVersion, len(resp.Responses[1].GetResponseRange().Kvs)) + for i, kv := range resp.Responses[1].GetResponseRange().Kvs { + var rules pb.Rules + err = proto.Unmarshal(kv.Value, &rules) + if err != nil { + return nil, errors.Wrap(err, "failed to unmarshal ruleset") + } - // decode rules - var rules pb.Rules - err = proto.Unmarshal(resp.Responses[2].GetResponseRange().Kvs[0].Value, &rules) - if err != nil { - return nil, errors.Wrap(err, "failed to unmarshal ruleset") - } - if version == "" { - _, version = s.pathVersionFromKey(string(resp.Responses[2].GetResponseRange().Kvs[0].Key)) + versions[len(versions)-i].Rules = rulesFromProtobuf(&rules) + _, versions[len(versions)-i].Version = s.pathVersionFromKey(string(kv.Key)) } return ®ula.Ruleset{ Path: path, - Version: version, - Rules: rulesFromProtobuf(&rules), Signature: signatureFromProtobuf(&sig), - Versions: versions.Versions, + Versions: versions, }, nil } diff --git a/api/service.go b/api/service.go index fe8937d..4b30618 100644 --- a/api/service.go +++ b/api/service.go @@ -24,9 +24,8 @@ type RulesetService interface { Create(ctx context.Context, path string, signature *regula.Signature) error // Put is used to add rules to a ruleset. It creates a new version of the ruleset. Put(ctx context.Context, path string, rules []*rule.Rule) (*regula.Ruleset, error) - // Get returns a ruleset alongside its metadata. By default, it returns the latest version. - // If the version is not empty, the specified version is returned. - Get(ctx context.Context, path, version string) (*regula.Ruleset, error) + // Get returns a ruleset alongside its metadata. + Get(ctx context.Context, path string) (*regula.Ruleset, error) // List returns the latest version of each ruleset whose path starts by the given prefix. // If the prefix is empty, it returns all the entries following the lexical order. // The listing is paginated and can be customised using the ListOptions type. From aac381afe95db051acbbe7473edce4f44b78ca3f Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 11:31:30 +0100 Subject: [PATCH 12/48] Simplify API Put --- api/etcd/put.go | 90 ++++++++++++------------------------------------- api/service.go | 4 +-- 2 files changed, 23 insertions(+), 71 deletions(-) diff --git a/api/etcd/put.go b/api/etcd/put.go index def975d..096e433 100644 --- a/api/etcd/put.go +++ b/api/etcd/put.go @@ -6,7 +6,6 @@ import ( "github.com/coreos/etcd/clientv3/concurrency" "github.com/golang/protobuf/proto" - "github.com/heetch/regula" "github.com/heetch/regula/api" pb "github.com/heetch/regula/api/etcd/proto" "github.com/heetch/regula/rule" @@ -15,22 +14,18 @@ import ( ) // Put stores the given rules under the rules tree. If no signature is found for the given path it returns an error. -func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) (*regula.Ruleset, error) { - var ruleset *regula.Ruleset - var err error - +func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) error { txfn := func(stm concurrency.STM) error { p := rulesPutter{s, stm} - ruleset, err = p.put(ctx, path, rules) - return err + return p.put(ctx, path, rules) } - _, err = concurrency.NewSTM(s.Client, txfn, concurrency.WithAbortContext(ctx)) + _, err := concurrency.NewSTM(s.Client, txfn, concurrency.WithAbortContext(ctx)) if err != nil && err != api.ErrRulesetNotModified && !api.IsValidationError(err) { - return nil, errors.Wrap(err, "failed to put ruleset") + return errors.Wrap(err, "failed to put ruleset") } - return ruleset, err + return err } // rulesPutter is responsible for validating and storing rules, updating checksums and other actions @@ -40,73 +35,54 @@ type rulesPutter struct { stm concurrency.STM } -func (p *rulesPutter) put(ctx context.Context, path string, rules []*rule.Rule) (*regula.Ruleset, error) { - var err error - - ruleset := regula.Ruleset{ - Path: path, - Rules: rules, - } - +func (p *rulesPutter) put(ctx context.Context, path string, rules []*rule.Rule) error { // validate the rules - ruleset.Signature, err = p.validateRules(p.stm, path, rules) + err := p.validateRules(p.stm, path, rules) if err != nil { - return nil, err + return err } // encode rules data, err := proto.Marshal(rulesToProtobuf(rules)) if err != nil { - return nil, err + return err } // update checksum if rules have changed changed, err := p.updateChecksum(p.stm, path, data) if err != nil { - return nil, err + return err } if !changed { - // fetch latest version string - _, ruleset.Version = p.s.pathVersionFromKey((p.stm.Get(p.s.latestVersionPath(path)))) - - return &ruleset, api.ErrRulesetNotModified - } - - // create a new version of the ruleset - ruleset.Version, err = p.createNewVersion(p.stm, path, data) - if err != nil { - return nil, err + return api.ErrRulesetNotModified } - // update the pointer to the latest ruleset version - p.stm.Put(p.s.latestVersionPath(path), p.s.rulesPath(path, ruleset.Version)) - - return &ruleset, p.updateVersionRegistry(p.stm, path, ruleset.Version) + // create a new version of the rules + return p.createNewVersion(p.stm, path, data) } // validateRules fetches the signature from the store and validates all the rules against it. -// if the rules are valid, it returns the signature. -func (p *rulesPutter) validateRules(stm concurrency.STM, path string, rules []*rule.Rule) (*regula.Signature, error) { +func (p *rulesPutter) validateRules(stm concurrency.STM, path string, rules []*rule.Rule) error { data := stm.Get(p.s.signaturesPath(path)) if data == "" { - return nil, api.ErrSignatureNotFound + return api.ErrSignatureNotFound } var pbsig pb.Signature err := proto.Unmarshal([]byte(data), &pbsig) if err != nil { - return nil, errors.Wrap(err, "failed to decode signature") + return errors.Wrap(err, "failed to decode signature") } sig := signatureFromProtobuf(&pbsig) for _, r := range rules { if err := api.ValidateRule(sig, r); err != nil { - return nil, err + return err } } - return sig, nil + return nil } // updateChecksum generates a checksum from the given data and stores it if it has changed. @@ -132,38 +108,14 @@ func (p *rulesPutter) updateChecksum(stm concurrency.STM, path string, data []by } // createNewVersion adds a new entry under /rulesets/rules//. -func (p *rulesPutter) createNewVersion(stm concurrency.STM, path string, data []byte) (string, error) { +func (p *rulesPutter) createNewVersion(stm concurrency.STM, path string, data []byte) error { // create a new ruleset version k, err := ksuid.NewRandom() if err != nil { - return "", errors.Wrap(err, "failed to generate rules version") + return errors.Wrap(err, "failed to generate rules version") } - version := k.String() - - stm.Put(p.s.rulesPath(path, version), string(data)) - - return version, nil -} - -// updateVersionRegistry stores the new version or appends it to the existing ones under the key /rulesets/versions/. -func (p *rulesPutter) updateVersionRegistry(stm concurrency.STM, path, version string) error { - var v pb.Versions - res := stm.Get(p.s.versionsPath(path)) - if res != "" { - err := proto.Unmarshal([]byte(res), &v) - if err != nil { - p.s.Logger.Debug().Err(err).Str("path", path).Msg("put: versions unmarshalling failed") - return errors.Wrap(err, "failed to unmarshal versions") - } - } - - v.Versions = append(v.Versions, version) - bvs, err := proto.Marshal(&v) - if err != nil { - return errors.Wrap(err, "failed to encode versions") - } - stm.Put(p.s.versionsPath(path), string(bvs)) + stm.Put(p.s.rulesPath(path, k.String()), string(data)) return nil } diff --git a/api/service.go b/api/service.go index 4b30618..2ea6dad 100644 --- a/api/service.go +++ b/api/service.go @@ -22,8 +22,8 @@ const ( type RulesetService interface { // Create a ruleset using a signature. Create(ctx context.Context, path string, signature *regula.Signature) error - // Put is used to add rules to a ruleset. It creates a new version of the ruleset. - Put(ctx context.Context, path string, rules []*rule.Rule) (*regula.Ruleset, error) + // Put is used to add a new version of the rules to a ruleset. + Put(ctx context.Context, path string, rules []*rule.Rule) error // Get returns a ruleset alongside its metadata. Get(ctx context.Context, path string) (*regula.Ruleset, error) // List returns the latest version of each ruleset whose path starts by the given prefix. From 7339a87fa3977daf95b6c956d546353c260549f3 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 11:36:20 +0100 Subject: [PATCH 13/48] API Put returns the new version string --- api/etcd/put.go | 24 ++++++++++++++---------- api/service.go | 2 +- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/api/etcd/put.go b/api/etcd/put.go index 096e433..af13028 100644 --- a/api/etcd/put.go +++ b/api/etcd/put.go @@ -15,9 +15,12 @@ import ( // Put stores the given rules under the rules tree. If no signature is found for the given path it returns an error. func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) error { + var version string txfn := func(stm concurrency.STM) error { p := rulesPutter{s, stm} - return p.put(ctx, path, rules) + var err error + version, err = p.put(ctx, path, rules) + return err } _, err := concurrency.NewSTM(s.Client, txfn, concurrency.WithAbortContext(ctx)) @@ -35,27 +38,27 @@ type rulesPutter struct { stm concurrency.STM } -func (p *rulesPutter) put(ctx context.Context, path string, rules []*rule.Rule) error { +func (p *rulesPutter) put(ctx context.Context, path string, rules []*rule.Rule) (string, error) { // validate the rules err := p.validateRules(p.stm, path, rules) if err != nil { - return err + return "", err } // encode rules data, err := proto.Marshal(rulesToProtobuf(rules)) if err != nil { - return err + return "", err } // update checksum if rules have changed changed, err := p.updateChecksum(p.stm, path, data) if err != nil { - return err + return "", err } if !changed { - return api.ErrRulesetNotModified + return "", api.ErrRulesetNotModified } // create a new version of the rules @@ -108,14 +111,15 @@ func (p *rulesPutter) updateChecksum(stm concurrency.STM, path string, data []by } // createNewVersion adds a new entry under /rulesets/rules//. -func (p *rulesPutter) createNewVersion(stm concurrency.STM, path string, data []byte) error { +func (p *rulesPutter) createNewVersion(stm concurrency.STM, path string, data []byte) (string, error) { // create a new ruleset version k, err := ksuid.NewRandom() if err != nil { - return errors.Wrap(err, "failed to generate rules version") + return "", errors.Wrap(err, "failed to generate rules version") } - stm.Put(p.s.rulesPath(path, k.String()), string(data)) + v := k.String() + stm.Put(p.s.rulesPath(path, v), string(data)) - return nil + return v, nil } diff --git a/api/service.go b/api/service.go index 2ea6dad..ea6c939 100644 --- a/api/service.go +++ b/api/service.go @@ -23,7 +23,7 @@ type RulesetService interface { // Create a ruleset using a signature. Create(ctx context.Context, path string, signature *regula.Signature) error // Put is used to add a new version of the rules to a ruleset. - Put(ctx context.Context, path string, rules []*rule.Rule) error + Put(ctx context.Context, path string, rules []*rule.Rule) (string, error) // Get returns a ruleset alongside its metadata. Get(ctx context.Context, path string) (*regula.Ruleset, error) // List returns the latest version of each ruleset whose path starts by the given prefix. From 5caccfe543881b0ca55dbea5600fd8dcab1eb77b Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 17:44:51 +0100 Subject: [PATCH 14/48] Add method to ListOptions --- api/service.go | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/api/service.go b/api/service.go index ea6c939..c6127b4 100644 --- a/api/service.go +++ b/api/service.go @@ -11,11 +11,11 @@ import ( // API errors. const ( - ErrRulesetNotFound = errors.Error("ruleset not found") - ErrRulesetNotModified = errors.Error("not modified") - ErrSignatureNotFound = errors.Error("signature not found") - ErrInvalidContinueToken = errors.Error("invalid continue token") - ErrAlreadyExists = errors.Error("already exists") + ErrRulesetNotFound = errors.Error("ruleset not found") + ErrRulesetNotModified = errors.Error("not modified") + ErrSignatureNotFound = errors.Error("signature not found") + ErrInvalidCursor = errors.Error("invalid cursor") + ErrAlreadyExists = errors.Error("already exists") ) // RulesetService is a service managing rulesets. @@ -26,8 +26,7 @@ type RulesetService interface { Put(ctx context.Context, path string, rules []*rule.Rule) (string, error) // Get returns a ruleset alongside its metadata. Get(ctx context.Context, path string) (*regula.Ruleset, error) - // List returns the latest version of each ruleset whose path starts by the given prefix. - // If the prefix is empty, it returns all the entries following the lexical order. + // List returns rulesets whose path starts by the given prefix. // The listing is paginated and can be customised using the ListOptions type. List(ctx context.Context, prefix string, opt *ListOptions) (*Rulesets, error) // Watch a prefix for changes and return a list of events. @@ -36,20 +35,27 @@ type RulesetService interface { Eval(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) } -// ListOptions contains list options. -// If the Limit is lower or equal to 0 or greater than 100, it will be set to 50 by default. +// ListOptions is used to customize the List output. type ListOptions struct { - Limit int - ContinueToken string - PathsOnly bool // return only the paths of the rulesets - AllVersions bool // return all versions of each rulesets + Limit int // If the Limit is lower or equal to 0 or greater than 100, it will be set to 50 by default. + Cursor string // pagination cursor + LatestVersionsLimit int // limit the number of versions to return. +} + +// GetLimit returns a limit that is withing the 0 - 100 boundries. +// If the limit is incorrect, it returns 50. +func (l *ListOptions) GetLimit() int { + if l.Limit <= 0 || l.Limit > 100 { + return 50 // TODO: make this one configurable + } + return l.Limit } // Rulesets holds a list of rulesets. type Rulesets struct { Rulesets []regula.Ruleset `json:"rulesets"` - Revision string `json:"revision"` // revision when the request was applied - Continue string `json:"continue,omitempty"` // token of the next page, if any + Revision string `json:"revision"` // revision when the request was applied + Cursor string `json:"cursor,omitempty"` // cursor of the next page, if any } // List of possible events executed against a ruleset. From ed103630e4cd8f74e4e9aa8075e65442ff05f127 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 17:49:59 +0100 Subject: [PATCH 15/48] Test GetLimit --- api/service_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 api/service_test.go diff --git a/api/service_test.go b/api/service_test.go new file mode 100644 index 0000000..02dc6f0 --- /dev/null +++ b/api/service_test.go @@ -0,0 +1,25 @@ +package api_test + +import ( + "testing" + + "github.com/heetch/regula/api" + "github.com/stretchr/testify/require" +) + +// Limit should be set to 50 if the given one is <= 0 or > 100. +func TestListOptionsGetLimit(t *testing.T) { + tests := []struct { + in, out int + }{ + {0, 50}, + {-10, 50}, + {110, 50}, + {70, 70}, + } + + for _, test := range tests { + opt := api.ListOptions{Limit: test.in} + require.Equal(t, test.out, opt.GetLimit()) + } +} From 99e3fe0e67d4415c670276dcc0227ea008c6e6d1 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 17:51:37 +0100 Subject: [PATCH 16/48] Rework list endpoint --- api/etcd/list.go | 185 +++++++---------------------- api/etcd/rulesets.go | 18 +-- api/etcd/rulesets_internal_test.go | 12 -- 3 files changed, 50 insertions(+), 165 deletions(-) diff --git a/api/etcd/list.go b/api/etcd/list.go index 04aac50..0bc6413 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -4,7 +4,6 @@ import ( "context" "encoding/base64" "strconv" - "strings" "github.com/coreos/etcd/clientv3" "github.com/gogo/protobuf/proto" @@ -14,61 +13,36 @@ import ( "github.com/pkg/errors" ) -func computeLimit(l int) int { - if l <= 0 || l > 100 { - return 50 // TODO: make this one configurable - } - return l -} - -// List returns the latest version of each ruleset under the given prefix. -// If the prefix is empty, it returns rulesets from the beginning following the lexical order. -// The listing can be customised using the ListOptions type. +// List returns rulesets whose path starts by the given prefix. +// The listing is paginated and can be customised using the ListOptions type. +// It runs two requests, one for fetching the signatures and one for fetching the related rules versions. func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListOptions) (*api.Rulesets, error) { - options := make([]clientv3.OpOption, 0, 3) + opts := make([]clientv3.OpOption, 0, 3) var key string - limit := computeLimit(opt.Limit) - - if opt.ContinueToken != "" { - lastPath, err := base64.URLEncoding.DecodeString(opt.ContinueToken) + // if a cursor is specified, decode the key from it and start the request from that key + if opt.Cursor != "" { + lastPath, err := base64.URLEncoding.DecodeString(opt.Cursor) if err != nil { - return nil, api.ErrInvalidContinueToken + return nil, api.ErrInvalidCursor } key = string(lastPath) - var rangeEnd string - if opt.AllVersions { - rangeEnd = clientv3.GetPrefixRangeEnd(s.rulesPath(prefix, "")) - } else { - rangeEnd = clientv3.GetPrefixRangeEnd(s.latestVersionPath(prefix)) - } - options = append(options, clientv3.WithRange(rangeEnd)) + opts = append(opts, clientv3.WithRange(clientv3.GetPrefixRangeEnd(s.rulesPath(prefix, "")))) } else { key = prefix - options = append(options, clientv3.WithPrefix()) + opts = append(opts, clientv3.WithPrefix()) } - options = append(options, clientv3.WithLimit(int64(limit))) - - switch { - case opt.PathsOnly: - return s.listPathsOnly(ctx, key, prefix, limit, options) - case opt.AllVersions: - return s.listAllVersions(ctx, key, prefix, limit, options) - default: - return s.listLastVersion(ctx, key, prefix, limit, options) - } -} + // limit the number of results + opts = append(opts, clientv3.WithLimit(int64(opt.GetLimit()))) -// listPathsOnly returns only the path for each ruleset. -func (s *RulesetService) listPathsOnly(ctx context.Context, key, prefix string, limit int, opts []clientv3.OpOption) (*api.Rulesets, error) { - opts = append(opts, clientv3.WithKeysOnly()) - resp, err := s.Client.KV.Get(ctx, s.latestVersionPath(key), opts...) + // fetch signatures + resp, err := s.Client.KV.Get(ctx, s.signaturesPath(key), opts...) if err != nil { - return nil, errors.Wrap(err, "failed to fetch versions") + return nil, errors.Wrap(err, "failed to fetch signatures") } // if a prefix is provided it must always return results @@ -77,127 +51,60 @@ func (s *RulesetService) listPathsOnly(ctx context.Context, key, prefix string, return nil, api.ErrRulesetNotFound } - rulesets := api.Rulesets{ - Rulesets: make([]regula.Ruleset, len(resp.Kvs)), - } - rulesets.Revision = strconv.FormatInt(resp.Header.Revision, 10) - for i, pair := range resp.Kvs { - p := strings.TrimPrefix(string(pair.Key), s.latestVersionPath("")+"/") - rulesets.Rulesets[i].Path = p - } - - if len(rulesets.Rulesets) < limit || !resp.More { - return &rulesets, nil - } + var ( + ops []clientv3.Op + rulesets api.Rulesets + ) - lastEntry := rulesets.Rulesets[len(rulesets.Rulesets)-1] - - // we want to start immediately after the last key - rulesets.Continue = base64.URLEncoding.EncodeToString([]byte(lastEntry.Path + "\x00")) - - return &rulesets, nil -} - -// listLastVersion returns only the latest version for each ruleset. -func (s *RulesetService) listLastVersion(ctx context.Context, key, prefix string, limit int, opts []clientv3.OpOption) (*api.Rulesets, error) { - resp, err := s.Client.KV.Get(ctx, s.latestVersionPath(key), opts...) - if err != nil { - return nil, errors.Wrap(err, "failed to fetch latest keys") - } - - // if a prefix is provided it must always return results - // otherwise it doesn't exist. - if resp.Count == 0 && prefix != "" { - return nil, api.ErrRulesetNotFound - } - - ops := make([]clientv3.Op, 0, resp.Count) - txn := s.Client.KV.Txn(ctx) - for _, pair := range resp.Kvs { - ops = append(ops, clientv3.OpGet(string(pair.Value))) - } - txnresp, err := txn.Then(ops...).Commit() - if err != nil { - return nil, errors.Wrap(err, "transaction failed to fetch selected rulesets") - } - - var rulesets api.Rulesets rulesets.Revision = strconv.FormatInt(resp.Header.Revision, 10) rulesets.Rulesets = make([]regula.Ruleset, len(resp.Kvs)) - // Responses handles responses for each OpGet calls in the transaction. - for i, resps := range txnresp.Responses { - var pbr pb.Rules - rr := resps.GetResponseRange() + for i, pair := range resp.Kvs { + var sig pb.Signature - // Given that we are getting a leaf in the tree (a ruleset entry), we are sure that we will always have one value in the Kvs slice. - err = proto.Unmarshal(rr.Kvs[0].Value, &pbr) + err = proto.Unmarshal(pair.Value, &sig) if err != nil { - s.Logger.Debug().Err(err).Bytes("entry", rr.Kvs[0].Value).Msg("list: unmarshalling failed") - return nil, errors.Wrap(err, "failed to unmarshal entry") - } - - path, version := s.pathVersionFromKey(string(rr.Kvs[0].Key)) - rulesets.Rulesets[i] = regula.Ruleset{ - Path: path, - Version: version, - Rules: rulesFromProtobuf(&pbr), + return nil, errors.Wrap(err, "failed to unmarshal signature") } - } - if len(rulesets.Rulesets) < limit || !resp.More { - return &rulesets, nil + rulesets.Rulesets[i].Path = s.pathFromKey("signatures", string(pair.Key)) + rulesets.Rulesets[i].Signature = signatureFromProtobuf(&sig) + ops = append(ops, clientv3.OpGet(s.rulesPath(rulesets.Rulesets[i].Path, ""), clientv3.WithLimit(int64(opt.LatestVersionsLimit)))) } - lastEntry := rulesets.Rulesets[len(rulesets.Rulesets)-1] - - // we want to start immediately after the last key - rulesets.Continue = base64.URLEncoding.EncodeToString([]byte(lastEntry.Path + "\x00")) - - return &rulesets, nil -} - -// listAllVersions returns all available versions for each ruleset. -func (s *RulesetService) listAllVersions(ctx context.Context, key, prefix string, limit int, opts []clientv3.OpOption) (*api.Rulesets, error) { - resp, err := s.Client.KV.Get(ctx, s.rulesPath(key, ""), opts...) + // running all the requests within a single transaction so only one network round trip is performed. + rulesResp, err := s.Client.KV.Txn(ctx).Then(ops...).Commit() if err != nil { - return nil, errors.Wrap(err, "failed to fetch all rulesets") - } - - // if a prefix is provided it must always return results - // otherwise it doesn't exist. - if resp.Count == 0 && prefix != "" { - return nil, api.ErrRulesetNotFound + return nil, errors.Wrapf(err, "failed to fetch rules") } - var rulesets api.Rulesets - rulesets.Revision = strconv.FormatInt(resp.Header.Revision, 10) - rulesets.Rulesets = make([]regula.Ruleset, len(resp.Kvs)) - for i, pair := range resp.Kvs { - var pbr pb.Rules - - err = proto.Unmarshal(pair.Value, &pbr) - if err != nil { - s.Logger.Debug().Err(err).Bytes("entry", pair.Value).Msg("list: unmarshalling failed") - return nil, errors.Wrap(err, "failed to unmarshal entry") + for i, rop := range rulesResp.Responses { + respRange := rop.GetResponseRange() + if respRange.Count == 0 { + continue } - path, version := s.pathVersionFromKey(string(pair.Key)) - rulesets.Rulesets[i] = regula.Ruleset{ - Path: path, - Version: version, - Rules: rulesFromProtobuf(&pbr), + rulesets.Rulesets[i].Versions = make([]regula.RulesetVersion, int(respRange.Count)) + for j, kv := range respRange.Kvs { + var pbr pb.Rules + err = proto.Unmarshal(kv.Value, &pbr) + if err != nil { + return nil, errors.Wrap(err, "failed to unmarshal rules") + } + + _, rulesets.Rulesets[i].Versions[j].Version = s.pathVersionFromKey(string(kv.Key)) + rulesets.Rulesets[i].Versions[j].Rules = rulesFromProtobuf(&pbr) } } - if len(rulesets.Rulesets) < limit || !resp.More { + if len(rulesets.Rulesets) < opt.GetLimit() || !resp.More { return &rulesets, nil } - lastEntry := rulesets.Rulesets[len(rulesets.Rulesets)-1] + lastRuleset := rulesets.Rulesets[len(rulesets.Rulesets)-1] // we want to start immediately after the last key - rulesets.Continue = base64.URLEncoding.EncodeToString([]byte(lastEntry.Path + versionSeparator + lastEntry.Version + "\x00")) + rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastRuleset.Path + "\x00")) return &rulesets, nil } diff --git a/api/etcd/rulesets.go b/api/etcd/rulesets.go index a62d3b8..d0df073 100644 --- a/api/etcd/rulesets.go +++ b/api/etcd/rulesets.go @@ -44,22 +44,12 @@ func (s *RulesetService) signaturesPath(p string) string { return path.Join(s.Namespace, "rulesets", "signatures", p) } -// latestVersionPath returns the path where the latest version string of each ruleset is stored in etcd. -// Key: /rulesets/latest/ -// Value: version string -func (s *RulesetService) latestVersionPath(p string) string { - return path.Join(s.Namespace, "rulesets", "latest", p) -} - -// versionsPath returns the path where the versions of each rulesets are stored in etcd. -// Key: /rulesets/versions/ -// Value: [version string, ] -func (s *RulesetService) versionsPath(p string) string { - return path.Join(s.Namespace, "rulesets", "versions", p) -} - func (s *RulesetService) pathVersionFromKey(key string) (string, string) { key = strings.TrimPrefix(key, path.Join(s.Namespace, "rulesets", "rules")+"/") chunks := strings.Split(key, versionSeparator) return chunks[0], chunks[1] } + +func (s *RulesetService) pathFromKey(collection, key string) string { + return strings.TrimPrefix(key, path.Join(s.Namespace, "rulesets", collection)+"/") +} diff --git a/api/etcd/rulesets_internal_test.go b/api/etcd/rulesets_internal_test.go index 802a650..f35cd5c 100644 --- a/api/etcd/rulesets_internal_test.go +++ b/api/etcd/rulesets_internal_test.go @@ -11,18 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -// Limit should be set to 50 if the given one is <= 0 or > 100. -func TestComputeLimit(t *testing.T) { - l := computeLimit(0) - require.Equal(t, 50, l) - l = computeLimit(-10) - require.Equal(t, 50, l) - l = computeLimit(110) - require.Equal(t, 50, l) - l = computeLimit(70) - require.Equal(t, 70, l) -} - // TestPathMethods ensures that the correct path are returned by each method. func TestPathMethods(t *testing.T) { s := &RulesetService{ From 6fc2c068751d18c3eacbea4114a25587d1d30183 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 28 Mar 2019 18:23:46 +0100 Subject: [PATCH 17/48] Fix Get test --- api/etcd/put.go | 6 +- api/etcd/rulesets_internal_test.go | 18 ++--- api/etcd/rulesets_test.go | 121 ++++++++++++++--------------- 3 files changed, 67 insertions(+), 78 deletions(-) diff --git a/api/etcd/put.go b/api/etcd/put.go index af13028..b880750 100644 --- a/api/etcd/put.go +++ b/api/etcd/put.go @@ -14,7 +14,7 @@ import ( ) // Put stores the given rules under the rules tree. If no signature is found for the given path it returns an error. -func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) error { +func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) (string, error) { var version string txfn := func(stm concurrency.STM) error { p := rulesPutter{s, stm} @@ -25,10 +25,10 @@ func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rul _, err := concurrency.NewSTM(s.Client, txfn, concurrency.WithAbortContext(ctx)) if err != nil && err != api.ErrRulesetNotModified && !api.IsValidationError(err) { - return errors.Wrap(err, "failed to put ruleset") + return "", errors.Wrap(err, "failed to put ruleset") } - return err + return version, err } // rulesPutter is responsible for validating and storing rules, updating checksums and other actions diff --git a/api/etcd/rulesets_internal_test.go b/api/etcd/rulesets_internal_test.go index f35cd5c..d8183e2 100644 --- a/api/etcd/rulesets_internal_test.go +++ b/api/etcd/rulesets_internal_test.go @@ -28,23 +28,17 @@ func TestPathMethods(t *testing.T) { exp = "test/rulesets/signatures/path" require.Equal(t, exp, s.signaturesPath("path")) - - exp = "test/rulesets/latest/path" - require.Equal(t, exp, s.latestVersionPath("path")) - - exp = "test/rulesets/versions/path" - require.Equal(t, exp, s.versionsPath("path")) } func BenchmarkProtoMarshalling(b *testing.B) { - rs := regula.NewRuleset( + rules := []*rule.Rule{ rule.New(rule.And(rule.Not(rule.BoolValue(false)), rule.BoolParam("param")), rule.BoolValue(true)), rule.New(rule.And(rule.BoolParam("1st-param"), rule.BoolParam("2nd-param")), rule.BoolValue(false)), - ) + } b.ResetTimer() for n := 0; n < b.N; n++ { - _, err := proto.Marshal(rulesToProtobuf(rs.Rules)) + _, err := proto.Marshal(rulesToProtobuf(rules)) require.NoError(b, err) } } @@ -63,12 +57,12 @@ func BenchmarkJSONMarshalling(b *testing.B) { } func BenchmarkProtoUnmarshalling(b *testing.B) { - rs := regula.NewRuleset( + rules := []*rule.Rule{ rule.New(rule.And(rule.Not(rule.BoolValue(false)), rule.BoolParam("param")), rule.BoolValue(true)), rule.New(rule.And(rule.BoolParam("1st-param"), rule.BoolParam("2nd-param")), rule.BoolValue(false)), - ) + } - bb, err := proto.Marshal(rulesToProtobuf(rs.Rules)) + bb, err := proto.Marshal(rulesToProtobuf(rules)) require.NoError(b, err) b.ResetTimer() diff --git a/api/etcd/rulesets_test.go b/api/etcd/rulesets_test.go index 31b1817..6588674 100644 --- a/api/etcd/rulesets_test.go +++ b/api/etcd/rulesets_test.go @@ -56,11 +56,14 @@ func newEtcdRulesetService(t *testing.T) (*etcd.RulesetService, func()) { } func createRuleset(t *testing.T, s *etcd.RulesetService, path string, rules ...*rule.Rule) *regula.Ruleset { - e, err := s.Put(context.Background(), path, rules) + _, err := s.Put(context.Background(), path, rules) if err != nil && err != api.ErrRulesetNotModified { require.NoError(t, err) } - return e + + rs, err := s.Get(context.Background(), path) + require.NoError(t, err) + return rs } func createBoolRuleset(t *testing.T, s *etcd.RulesetService, path string, rules ...*rule.Rule) *regula.Ruleset { @@ -79,44 +82,36 @@ func TestGet(t *testing.T) { sig := ®ula.Signature{ReturnType: "bool", Params: make(map[string]string)} t.Run("Root", func(t *testing.T) { - rs1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - createBoolRuleset(t, s, path, rs1...) + rules1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + createBoolRuleset(t, s, path, rules1...) - entry1, err := s.Get(context.Background(), path, "") + ruleset1, err := s.Get(context.Background(), path) require.NoError(t, err) - require.Equal(t, path, entry1.Path) - require.Equal(t, rs1, entry1.Rules) - require.Equal(t, sig, entry1.Signature) - require.NotEmpty(t, entry1.Version) - require.Len(t, entry1.Versions, 1) + require.Equal(t, path, ruleset1.Path) + require.Len(t, ruleset1.Versions, 1) + require.Equal(t, rules1, ruleset1.Versions[0].Rules) + require.NotEmpty(t, rules1, ruleset1.Versions[0].Version) + require.Equal(t, sig, ruleset1.Signature) + require.Len(t, ruleset1.Versions, 1) // we are waiting 1 second here to avoid creating the new version in the same second as the previous one // ksuid gives a sorting with a one second precision time.Sleep(time.Second) - rs2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("foo")), rule.BoolValue(true))} - createBoolRuleset(t, s, path, rs2...) - - // by default, it should return the latest version - entry2, err := s.Get(context.Background(), path, "") - require.NoError(t, err) - require.Equal(t, path, entry2.Path) - require.Equal(t, rs2, entry2.Rules) - require.Equal(t, sig, entry2.Signature) - require.NotEmpty(t, entry2.Version) - require.Len(t, entry2.Versions, 2) - - // get a specific version - entry3, err := s.Get(context.Background(), path, entry1.Version) - require.NoError(t, err) - require.Equal(t, entry1.Path, entry3.Path) - require.Equal(t, entry1.Rules, entry3.Rules) - require.Equal(t, entry1.Signature, entry3.Signature) - require.Equal(t, entry1.Version, entry3.Version) - require.Len(t, entry3.Versions, 2) + rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("foo")), rule.BoolValue(true))} + createBoolRuleset(t, s, path, rules2...) + + // it should return two versions, in ascending order + ruleset2, err := s.Get(context.Background(), path) + require.NoError(t, err) + require.Equal(t, path, ruleset2.Path) + require.Len(t, ruleset1.Versions, 2) + require.Equal(t, rules1, ruleset1.Versions[0].Rules) + require.Equal(t, rules2, ruleset1.Versions[1].Rules) + require.Equal(t, sig, ruleset2.Signature) }) t.Run("Not found", func(t *testing.T) { - _, err := s.Get(context.Background(), "doesntexist", "") + _, err := s.Get(context.Background(), "doesntexist") require.Equal(t, err, api.ErrRulesetNotFound) }) } @@ -154,33 +149,33 @@ func TestList(t *testing.T) { // Assert that only latest version for each ruleset is returned by default. t.Run("Last version only", func(t *testing.T) { prefix := "list/last/version/" - rs1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} - rs2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} + rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} + rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} createBoolRuleset(t, s, prefix+"a", rs...) createBoolRuleset(t, s, prefix+"a/1", rs...) - createBoolRuleset(t, s, prefix+"a", rs1...) - createBoolRuleset(t, s, prefix+"a", rs2...) + createBoolRuleset(t, s, prefix+"a", rules1...) + createBoolRuleset(t, s, prefix+"a", rules2...) entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) require.NoError(t, err) require.Len(t, entries.Rulesets, 2) a := entries.Rulesets[0] - require.Equal(t, rs2, a.Rules) + require.Equal(t, rules2, a.Rules) require.NotEmpty(t, entries.Revision) }) // Assert that all versions are returned when passing the AllVersions option. t.Run("All versions", func(t *testing.T) { prefix := "list/all/version/" - rs1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} - rs2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} + rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} + rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} createBoolRuleset(t, s, prefix+"a", rs...) time.Sleep(time.Second) - createBoolRuleset(t, s, prefix+"a", rs1...) + createBoolRuleset(t, s, prefix+"a", rules1...) time.Sleep(time.Second) - createBoolRuleset(t, s, prefix+"a", rs2...) + createBoolRuleset(t, s, prefix+"a", rules2...) createBoolRuleset(t, s, prefix+"a/1", rs...) paths := []string{prefix + "a", prefix + "a", prefix + "a", prefix + "a/1"} @@ -204,7 +199,7 @@ func TestList(t *testing.T) { require.Equal(t, prefix+"a", entries.Rulesets[0].Path) require.Equal(t, rs, entries.Rulesets[0].Rules) require.Equal(t, prefix+"a", entries.Rulesets[1].Path) - require.Equal(t, rs1, entries.Rulesets[1].Rules) + require.Equal(t, rules1, entries.Rulesets[1].Rules) require.NotEmpty(t, entries.Revision) opt.ContinueToken = entries.Continue @@ -212,7 +207,7 @@ func TestList(t *testing.T) { require.NoError(t, err) require.Len(t, entries.Rulesets, opt.Limit) require.Equal(t, prefix+"a", entries.Rulesets[0].Path) - require.Equal(t, rs2, entries.Rulesets[0].Rules) + require.Equal(t, rules2, entries.Rulesets[0].Rules) require.Equal(t, prefix+"a/1", entries.Rulesets[1].Path) require.Equal(t, rs, entries.Rulesets[1].Rules) require.NotEmpty(t, entries.Revision) @@ -448,18 +443,18 @@ func TestPut(t *testing.T) { path := "a" rs := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - entry, err := s.Put(context.Background(), path, rs) + ruleset, err := s.Put(context.Background(), path, rs) require.NoError(t, err) - require.Equal(t, path, entry.Path) - require.NotEmpty(t, entry.Version) - require.Equal(t, rs, entry.Rules) + require.Equal(t, path, ruleset.Path) + require.NotEmpty(t, ruleset.Version) + require.Equal(t, rs, ruleset.Rules) // verify ruleset creation resp, err := s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "rules", path), clientv3.WithPrefix()) require.NoError(t, err) require.EqualValues(t, 1, resp.Count) // verify if the path contains the right ruleset version - require.Equal(t, entry.Version, strings.TrimPrefix(string(resp.Kvs[0].Key), ppath.Join(s.Namespace, "rulesets", "rules", "a")+"!")) + require.Equal(t, ruleset.Version, strings.TrimPrefix(string(resp.Kvs[0].Key), ppath.Join(s.Namespace, "rulesets", "rules", "a")+"!")) // verify checksum creation resp, err = s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "checksums", path), clientv3.WithPrefix()) @@ -480,19 +475,19 @@ func TestPut(t *testing.T) { err = proto.Unmarshal(resp.Kvs[0].Value, &v) require.NoError(t, err) require.Len(t, v.Versions, 1) - require.EqualValues(t, entry.Version, v.Versions[0]) + require.EqualValues(t, ruleset.Version, v.Versions[0]) // create new version with same ruleset - entry2, err := s.Put(context.Background(), path, rs) + ruleset2, err := s.Put(context.Background(), path, rs) require.Equal(t, api.ErrRulesetNotModified, err) - require.Equal(t, entry, entry2) + require.Equal(t, ruleset, ruleset2) // create new version with different ruleset rs = []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} - entry2, err = s.Put(context.Background(), path, rs) + ruleset2, err = s.Put(context.Background(), path, rs) require.NoError(t, err) - require.NotEqual(t, entry.Version, entry2.Version) + require.NotEqual(t, ruleset.Version, ruleset2.Version) // verify versions list append resp, err = s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "versions", path), clientv3.WithPrefix()) @@ -502,15 +497,15 @@ func TestPut(t *testing.T) { err = proto.Unmarshal(resp.Kvs[0].Value, &v) require.NoError(t, err) require.Len(t, v.Versions, 2) - require.EqualValues(t, entry.Version, v.Versions[0]) - require.EqualValues(t, entry2.Version, v.Versions[1]) + require.EqualValues(t, ruleset.Version, v.Versions[0]) + require.EqualValues(t, ruleset2.Version, v.Versions[1]) }) t.Run("Signatures", func(t *testing.T) { path := "b" require.NoError(t, s.Create(context.Background(), path, ®ula.Signature{ReturnType: "bool", Params: map[string]string{"a": "string", "b": "bool", "c": "int64"}})) - rs1 := []*rule.Rule{ + rules1 := []*rule.Rule{ rule.New( rule.Eq( rule.StringParam("a"), @@ -521,11 +516,11 @@ func TestPut(t *testing.T) { ), } - _, err := s.Put(context.Background(), path, rs1) + _, err := s.Put(context.Background(), path, rules1) require.NoError(t, err) // same params, different return type - rs2 := []*rule.Rule{ + rules2 := []*rule.Rule{ rule.New( rule.Eq( rule.StringParam("a"), @@ -536,7 +531,7 @@ func TestPut(t *testing.T) { ), } - _, err = s.Put(context.Background(), path, rs2) + _, err = s.Put(context.Background(), path, rules2) require.True(t, api.IsValidationError(err)) // adding new params @@ -687,14 +682,14 @@ func TestEval(t *testing.T) { rule.BoolValue(true), ) - entry := createRuleset(t, s, "a", r) + ruleset := createRuleset(t, s, "a", r) t.Run("OK", func(t *testing.T) { - res, err := s.Eval(context.Background(), "a", entry.Version, regula.Params{ + res, err := s.Eval(context.Background(), "a", ruleset.Version, regula.Params{ "id": "123", }) require.NoError(t, err) - require.Equal(t, entry.Version, res.Version) + require.Equal(t, ruleset.Version, res.Version) require.Equal(t, rule.BoolValue(true), res.Value) }) @@ -703,12 +698,12 @@ func TestEval(t *testing.T) { "id": "123", }) require.NoError(t, err) - require.Equal(t, entry.Version, res.Version) + require.Equal(t, ruleset.Version, res.Version) require.Equal(t, rule.BoolValue(true), res.Value) }) t.Run("NotFound", func(t *testing.T) { - _, err := s.Eval(context.Background(), "b", entry.Version, regula.Params{ + _, err := s.Eval(context.Background(), "b", ruleset.Version, regula.Params{ "id": "123", }) require.Equal(t, errors.ErrRulesetNotFound, err) From b9f18c1e5b1ff09ccc142d4faafb0ed512004f9c Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 29 Mar 2019 10:28:04 +0100 Subject: [PATCH 18/48] Add LatestVersion method to Ruleset --- ruleset.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ruleset.go b/ruleset.go index 97f87b6..9bc4e70 100644 --- a/ruleset.go +++ b/ruleset.go @@ -32,7 +32,7 @@ func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { return nil, rerrors.ErrNoMatch } - for _, rl := range r.Versions[len(r.Versions)-1].Rules { + for _, rl := range r.LatestVersion().Rules { res, err := rl.Eval(params) if err != rerrors.ErrNoMatch { return res, err @@ -63,6 +63,16 @@ func (r *Ruleset) EvalVersion(version string, params rule.Params) (*rule.Value, return nil, rerrors.ErrNoMatch } +// LatestVersion returns the latest RuleserVersion stored in the ruleset +// or nil if there are none. +func (r *Ruleset) LatestVersion() *RulesetVersion { + if len(r.Versions) == 0 { + return nil + } + + return &r.Versions[len(r.Versions)-1] +} + // Signature represents the signature of a ruleset. type Signature struct { ReturnType string `json:"returnType"` From d77902fe0e3eacceec186d9532e7f5fdd9285104 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Mon, 8 Apr 2019 18:05:46 +0200 Subject: [PATCH 19/48] Split etcd List method --- api/etcd/list.go | 55 +++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/api/etcd/list.go b/api/etcd/list.go index 0bc6413..5abda8a 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -19,6 +19,10 @@ import ( func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListOptions) (*api.Rulesets, error) { opts := make([]clientv3.OpOption, 0, 3) + if opt == nil { + opt = new(api.ListOptions) + } + var key string // if a cursor is specified, decode the key from it and start the request from that key @@ -51,13 +55,12 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO return nil, api.ErrRulesetNotFound } - var ( - ops []clientv3.Op - rulesets api.Rulesets - ) + var ops []clientv3.Op - rulesets.Revision = strconv.FormatInt(resp.Header.Revision, 10) - rulesets.Rulesets = make([]regula.Ruleset, len(resp.Kvs)) + rulesets := api.Rulesets{ + Revision: strconv.FormatInt(resp.Header.Revision, 10), + Rulesets: make([]regula.Ruleset, len(resp.Kvs)), + } for i, pair := range resp.Kvs { var sig pb.Signature @@ -69,13 +72,36 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO rulesets.Rulesets[i].Path = s.pathFromKey("signatures", string(pair.Key)) rulesets.Rulesets[i].Signature = signatureFromProtobuf(&sig) - ops = append(ops, clientv3.OpGet(s.rulesPath(rulesets.Rulesets[i].Path, ""), clientv3.WithLimit(int64(opt.LatestVersionsLimit)))) + + if opt.LatestVersionsLimit > 0 { + ops = append(ops, clientv3.OpGet(s.rulesPath(rulesets.Rulesets[i].Path, ""), clientv3.WithLimit(int64(opt.LatestVersionsLimit)))) + } + } + + if len(ops) > 0 { + err = s.fetchRulesVersions(ctx, &rulesets, ops) + if err != nil { + return nil, err + } + } + + if len(rulesets.Rulesets) < opt.GetLimit() || !resp.More { + return &rulesets, nil } + lastRuleset := rulesets.Rulesets[len(rulesets.Rulesets)-1] + + // we want to start immediately after the last key + rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastRuleset.Path + "\x00")) + + return &rulesets, nil +} + +func (s *RulesetService) fetchRulesVersions(ctx context.Context, rulesets *api.Rulesets, ops []clientv3.Op) error { // running all the requests within a single transaction so only one network round trip is performed. rulesResp, err := s.Client.KV.Txn(ctx).Then(ops...).Commit() if err != nil { - return nil, errors.Wrapf(err, "failed to fetch rules") + return errors.Wrapf(err, "failed to fetch rules") } for i, rop := range rulesResp.Responses { @@ -89,7 +115,7 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO var pbr pb.Rules err = proto.Unmarshal(kv.Value, &pbr) if err != nil { - return nil, errors.Wrap(err, "failed to unmarshal rules") + return errors.Wrap(err, "failed to unmarshal rules") } _, rulesets.Rulesets[i].Versions[j].Version = s.pathVersionFromKey(string(kv.Key)) @@ -97,14 +123,5 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO } } - if len(rulesets.Rulesets) < opt.GetLimit() || !resp.More { - return &rulesets, nil - } - - lastRuleset := rulesets.Rulesets[len(rulesets.Rulesets)-1] - - // we want to start immediately after the last key - rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastRuleset.Path + "\x00")) - - return &rulesets, nil + return nil } From df06ca53019e1f5e1a0ec0226c1dbc9d9e77e016 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 7 May 2019 15:29:02 +0200 Subject: [PATCH 20/48] Simplify list implementation --- api/etcd/list.go | 55 ++++++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/api/etcd/list.go b/api/etcd/list.go index 5abda8a..5dadb97 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -15,7 +15,7 @@ import ( // List returns rulesets whose path starts by the given prefix. // The listing is paginated and can be customised using the ListOptions type. -// It runs two requests, one for fetching the signatures and one for fetching the related rules versions. +// It runs two requests to etcd, one for fetching the signatures and one for fetching the related rules versions. func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListOptions) (*api.Rulesets, error) { opts := make([]clientv3.OpOption, 0, 3) @@ -55,8 +55,37 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO return nil, api.ErrRulesetNotFound } - var ops []clientv3.Op + // decode signatures into rulesets + rulesets, err := s.decodeSignatures(resp) + if err != nil { + return nil, err + } + + // if the user also wants the associated rules, we add an operation for every signature. + if opt.LatestVersionsLimit > 0 { + ops := make([]clientv3.Op, len(resp.Kvs)) + for i := range resp.Kvs { + ops[i] = clientv3.OpGet(s.rulesPath(rulesets.Rulesets[i].Path, ""), clientv3.WithLimit(int64(opt.LatestVersionsLimit))) + } + err = s.fetchRulesVersions(ctx, rulesets, ops) + if err != nil { + return nil, err + } + } + + // if there are still rulesets left, generate a new cursor + if len(rulesets.Rulesets) == opt.GetLimit() && resp.More { + lastRuleset := rulesets.Rulesets[len(rulesets.Rulesets)-1] + + // we want to start immediately after the last key + rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastRuleset.Path + "\x00")) + } + + return rulesets, nil +} + +func (s *RulesetService) decodeSignatures(resp *clientv3.GetResponse) (*api.Rulesets, error) { rulesets := api.Rulesets{ Revision: strconv.FormatInt(resp.Header.Revision, 10), Rulesets: make([]regula.Ruleset, len(resp.Kvs)), @@ -65,35 +94,15 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO for i, pair := range resp.Kvs { var sig pb.Signature - err = proto.Unmarshal(pair.Value, &sig) + err := proto.Unmarshal(pair.Value, &sig) if err != nil { return nil, errors.Wrap(err, "failed to unmarshal signature") } rulesets.Rulesets[i].Path = s.pathFromKey("signatures", string(pair.Key)) rulesets.Rulesets[i].Signature = signatureFromProtobuf(&sig) - - if opt.LatestVersionsLimit > 0 { - ops = append(ops, clientv3.OpGet(s.rulesPath(rulesets.Rulesets[i].Path, ""), clientv3.WithLimit(int64(opt.LatestVersionsLimit)))) - } - } - - if len(ops) > 0 { - err = s.fetchRulesVersions(ctx, &rulesets, ops) - if err != nil { - return nil, err - } - } - - if len(rulesets.Rulesets) < opt.GetLimit() || !resp.More { - return &rulesets, nil } - lastRuleset := rulesets.Rulesets[len(rulesets.Rulesets)-1] - - // we want to start immediately after the last key - rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastRuleset.Path + "\x00")) - return &rulesets, nil } From 1e2340957f9ad570e100a832cacf9d2969be6288 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 7 May 2019 16:10:04 +0200 Subject: [PATCH 21/48] Extract put tests --- api/etcd/put_test.go | 185 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 api/etcd/put_test.go diff --git a/api/etcd/put_test.go b/api/etcd/put_test.go new file mode 100644 index 0000000..c371e74 --- /dev/null +++ b/api/etcd/put_test.go @@ -0,0 +1,185 @@ +package etcd + +import ( + "context" + ppath "path" + "strings" + "testing" + + "github.com/coreos/etcd/clientv3" + "github.com/gogo/protobuf/proto" + "github.com/heetch/regula" + "github.com/heetch/regula/api" + pb "github.com/heetch/regula/api/etcd/proto" + "github.com/heetch/regula/rule" + "github.com/stretchr/testify/require" +) + +func TestPut(t *testing.T) { + t.Parallel() + + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + path := "a" + sig := ®ula.Signature{ReturnType: "bool"} + require.NoError(t, s.Create(context.Background(), path, sig)) + + t.Run("OK", func(t *testing.T) { + path := "a" + rules := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + + // put the rules + version, err := s.Put(context.Background(), path, rules) + require.NoError(t, err) + require.NotEmpty(t, version) + + // verify rules creation + resp, err := s.Client.Get(context.Background(), s.rulesPath(path, ""), clientv3.WithPrefix()) + require.NoError(t, err) + require.EqualValues(t, 1, resp.Count) + var pbrules pb.Rules + err = proto.Unmarshal(resp.Kvs[0].Value, &pbrules) + require.EqualValues(t, rules, rulesFromProtobuf(&pbrules)) + + // verify if the path contains the right rules version + require.Equal(t, version, strings.TrimPrefix(string(resp.Kvs[0].Key), ppath.Join(s.Namespace, "rulesets", "rules", "a")+"!")) + + // verify checksum creation + resp, err = s.Client.Get(context.Background(), s.checksumsPath(path), clientv3.WithPrefix()) + require.NoError(t, err) + require.EqualValues(t, 1, resp.Count) + + // create new version with same ruleset + newVersion, err := s.Put(context.Background(), path, rules) + require.Equal(t, api.ErrRulesetNotModified, err) + require.Equal(t, version, newVersion) + + // create new version with different rules + rules = []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} + + newVersion, err = s.Put(context.Background(), path, rules) + require.NoError(t, err) + require.NotEqual(t, version, newVersion) + + // verify new rules creation + resp, err = s.Client.Get(context.Background(), s.rulesPath(path, newVersion), clientv3.WithPrefix()) + require.NoError(t, err) + require.EqualValues(t, 1, resp.Count) + err = proto.Unmarshal(resp.Kvs[0].Value, &pbrules) + require.NoError(t, err) + require.EqualValues(t, rules, rulesFromProtobuf(&pbrules)) + }) + + t.Run("Signature checks", func(t *testing.T) { + path := "b" + require.NoError(t, s.Create(context.Background(), path, ®ula.Signature{ReturnType: "bool", Params: map[string]string{"a": "string", "b": "bool", "c": "int64"}})) + + rules1 := []*rule.Rule{ + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.BoolParam("b"), + rule.Int64Param("c"), + ), + rule.BoolValue(true), + ), + } + + _, err := s.Put(context.Background(), path, rules1) + require.NoError(t, err) + + // same params, different return type + rules2 := []*rule.Rule{ + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.BoolParam("b"), + rule.Int64Param("c"), + ), + rule.StringValue("true"), + ), + } + + _, err = s.Put(context.Background(), path, rules2) + require.True(t, api.IsValidationError(err)) + + // adding new params + rs3 := []*rule.Rule{ + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.BoolParam("b"), + rule.Int64Param("c"), + rule.BoolParam("d"), + ), + rule.BoolValue(true), + ), + } + + _, err = s.Put(context.Background(), path, rs3) + require.True(t, api.IsValidationError(err)) + + // changing param types + rs4 := []*rule.Rule{ + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.StringParam("b"), + rule.Int64Param("c"), + rule.BoolParam("d"), + ), + rule.BoolValue(true), + ), + } + + _, err = s.Put(context.Background(), path, rs4) + require.True(t, api.IsValidationError(err)) + + // adding new rule with different param types + rs5 := []*rule.Rule{ + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.StringParam("b"), + rule.Int64Param("c"), + rule.BoolParam("d"), + ), + rule.BoolValue(true), + ), + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.StringParam("b"), + rule.Int64Param("c"), + rule.BoolParam("d"), + ), + rule.BoolValue(true), + ), + } + + _, err = s.Put(context.Background(), path, rs5) + require.True(t, api.IsValidationError(err)) + + // adding new rule with correct param types but less + rs6 := []*rule.Rule{ + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.BoolParam("b"), + ), + rule.BoolValue(true), + ), + rule.New( + rule.Eq( + rule.StringParam("a"), + rule.BoolParam("b"), + ), + rule.BoolValue(true), + ), + } + + _, err = s.Put(context.Background(), path, rs6) + require.NoError(t, err) + }) +} From a691b685f552664d28c15c059c62bceb17eaeeb2 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 7 May 2019 16:12:32 +0200 Subject: [PATCH 22/48] Extract get tests --- api/etcd/get_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 api/etcd/get_test.go diff --git a/api/etcd/get_test.go b/api/etcd/get_test.go new file mode 100644 index 0000000..1e50450 --- /dev/null +++ b/api/etcd/get_test.go @@ -0,0 +1,56 @@ +package etcd + +import ( + "context" + "testing" + "time" + + "github.com/heetch/regula" + "github.com/heetch/regula/api" + "github.com/heetch/regula/rule" + "github.com/stretchr/testify/require" +) + +func TestGet(t *testing.T) { + t.Parallel() + + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + path := "p/a/t/h" + sig := ®ula.Signature{ReturnType: "bool", Params: make(map[string]string)} + + t.Run("Root", func(t *testing.T) { + rules1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + createBoolRuleset(t, s, path, rules1...) + + ruleset1, err := s.Get(context.Background(), path) + require.NoError(t, err) + require.Equal(t, path, ruleset1.Path) + require.Len(t, ruleset1.Versions, 1) + require.Equal(t, rules1, ruleset1.Versions[0].Rules) + require.NotEmpty(t, rules1, ruleset1.Versions[0].Version) + require.Equal(t, sig, ruleset1.Signature) + require.Len(t, ruleset1.Versions, 1) + + // we are waiting 1 second here to avoid creating the new version in the same second as the previous one + // ksuid gives a sorting with a one second precision + time.Sleep(time.Second) + rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("foo")), rule.BoolValue(true))} + createBoolRuleset(t, s, path, rules2...) + + // it should return two versions, in ascending order + ruleset2, err := s.Get(context.Background(), path) + require.NoError(t, err) + require.Equal(t, path, ruleset2.Path) + require.Len(t, ruleset1.Versions, 2) + require.Equal(t, rules1, ruleset1.Versions[0].Rules) + require.Equal(t, rules2, ruleset1.Versions[1].Rules) + require.Equal(t, sig, ruleset2.Signature) + }) + + t.Run("Not found", func(t *testing.T) { + _, err := s.Get(context.Background(), "doesntexist") + require.Equal(t, err, api.ErrRulesetNotFound) + }) +} From 09a7b5a1869fdf94f5e89aa20788c5e245910d8a Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 7 May 2019 16:13:41 +0200 Subject: [PATCH 23/48] Extract watch tests --- api/etcd/watch_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 api/etcd/watch_test.go diff --git a/api/etcd/watch_test.go b/api/etcd/watch_test.go new file mode 100644 index 0000000..565f05a --- /dev/null +++ b/api/etcd/watch_test.go @@ -0,0 +1,63 @@ +package etcd + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/heetch/regula/api" + "github.com/heetch/regula/rule" + "github.com/stretchr/testify/require" +) + +func TestWatch(t *testing.T) { + t.Parallel() + + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + var wg sync.WaitGroup + + wg.Add(1) + go func() { + defer wg.Done() + + time.Sleep(time.Second) + + r := rule.New(rule.True(), rule.BoolValue(true)) + + createBoolRuleset(t, s, "aa", r) + createBoolRuleset(t, s, "ab", r) + createBoolRuleset(t, s, "a/1", r) + }() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + events, err := s.Watch(ctx, "a", "") + require.NoError(t, err) + require.Len(t, events.Events, 1) + require.NotEmpty(t, events.Revision) + require.Equal(t, "aa", events.Events[0].Path) + require.Equal(t, api.RulesetPutEvent, events.Events[0].Type) + + wg.Wait() + + events, err = s.Watch(ctx, "a", events.Revision) + require.NoError(t, err) + require.Len(t, events.Events, 2) + require.NotEmpty(t, events.Revision) + require.Equal(t, api.RulesetPutEvent, events.Events[0].Type) + require.Equal(t, "ab", events.Events[0].Path) + require.Equal(t, api.RulesetPutEvent, events.Events[1].Type) + require.Equal(t, "a/1", events.Events[1].Path) + + t.Run("timeout", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) + defer cancel() + events, err := s.Watch(ctx, "", "") + require.Equal(t, context.DeadlineExceeded, err) + require.True(t, events.Timeout) + }) +} From 880e4a5c7dd77cc149d74a13f5500c5d47a061cc Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 7 May 2019 16:16:59 +0200 Subject: [PATCH 24/48] Extract eval tests --- api/etcd/eval_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 api/etcd/eval_test.go diff --git a/api/etcd/eval_test.go b/api/etcd/eval_test.go new file mode 100644 index 0000000..ca2bdd7 --- /dev/null +++ b/api/etcd/eval_test.go @@ -0,0 +1,64 @@ +package etcd + +import ( + "context" + "testing" + + "github.com/heetch/regula" + "github.com/heetch/regula/errors" + "github.com/heetch/regula/rule" + "github.com/stretchr/testify/require" +) + +func TestEval(t *testing.T) { + t.Parallel() + + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + sig := ®ula.Signature{ReturnType: "bool", Params: map[string]string{"id": "string"}} + require.NoError(t, s.Create(context.Background(), "a", sig)) + + r := rule.New( + rule.Eq( + rule.StringParam("id"), + rule.StringValue("123"), + ), + rule.BoolValue(true), + ) + + ruleset := createRuleset(t, s, "a", r) + version := ruleset.LatestVersion().Version + + t.Run("OK", func(t *testing.T) { + res, err := s.Eval(context.Background(), "a", version, regula.Params{ + "id": "123", + }) + require.NoError(t, err) + require.Equal(t, version, res.Version) + require.Equal(t, rule.BoolValue(true), res.Value) + }) + + t.Run("Latest", func(t *testing.T) { + res, err := s.Eval(context.Background(), "a", "", regula.Params{ + "id": "123", + }) + require.NoError(t, err) + require.Equal(t, version, res.Version) + require.Equal(t, rule.BoolValue(true), res.Value) + }) + + t.Run("NotFound", func(t *testing.T) { + _, err := s.Eval(context.Background(), "b", version, regula.Params{ + "id": "123", + }) + require.Equal(t, errors.ErrRulesetNotFound, err) + }) + + t.Run("BadVersion", func(t *testing.T) { + _, err := s.Eval(context.Background(), "a", "someversion", regula.Params{ + "id": "123", + }) + require.Equal(t, errors.ErrRulesetNotFound, err) + }) +} From 60426f109487b2af0221ff0cc47ef62733757e47 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 15:24:45 +0200 Subject: [PATCH 25/48] Change List signature --- api/service.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api/service.go b/api/service.go index c6127b4..3e71298 100644 --- a/api/service.go +++ b/api/service.go @@ -26,9 +26,9 @@ type RulesetService interface { Put(ctx context.Context, path string, rules []*rule.Rule) (string, error) // Get returns a ruleset alongside its metadata. Get(ctx context.Context, path string) (*regula.Ruleset, error) - // List returns rulesets whose path starts by the given prefix. + // List returns the list of all rulesets paths. // The listing is paginated and can be customised using the ListOptions type. - List(ctx context.Context, prefix string, opt *ListOptions) (*Rulesets, error) + List(ctx context.Context, opt ListOptions) (*Rulesets, error) // Watch a prefix for changes and return a list of events. Watch(ctx context.Context, prefix string, revision string) (*RulesetEvents, error) // Eval evaluates a ruleset given a path and a set of parameters. It implements the regula.Evaluator interface. @@ -53,9 +53,9 @@ func (l *ListOptions) GetLimit() int { // Rulesets holds a list of rulesets. type Rulesets struct { - Rulesets []regula.Ruleset `json:"rulesets"` - Revision string `json:"revision"` // revision when the request was applied - Cursor string `json:"cursor,omitempty"` // cursor of the next page, if any + Paths []string `json:"paths"` + Revision string `json:"revision"` // revision when the request was applied + Cursor string `json:"cursor,omitempty"` // cursor of the next page, if any } // List of possible events executed against a ruleset. From db2675cafa278a99d0d1bbe83156f5ecaddfbb82 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 15:41:04 +0200 Subject: [PATCH 26/48] Simplifyied ruleset definition --- ruleset.go | 64 +++++++++--------------------------------------------- 1 file changed, 10 insertions(+), 54 deletions(-) diff --git a/ruleset.go b/ruleset.go index 9bc4e70..958e269 100644 --- a/ruleset.go +++ b/ruleset.go @@ -8,71 +8,33 @@ import ( // A Ruleset is a list of rules and their metadata. type Ruleset struct { - Path string `json:"path"` - Signature *Signature `json:"signature,omitempty"` - Versions []RulesetVersion `json:"versions,omitempty"` + Path string `json:"path"` + Version string `json:"version,omitempty"` + Rules []*rule.Rule `json:"rules,omitempty"` + Signature *Signature `json:"signature,omitempty"` + Versions []string `json:"versions,omitempty"` } // NewRuleset creates a ruleset. func NewRuleset(rules ...*rule.Rule) *Ruleset { - var rs Ruleset - - rs.Versions = append(rs.Versions, RulesetVersion{ - Version: "latest", - Rules: rules, - }) - - return &rs + return &Ruleset{ + Rules: rules, + } } -// Eval evaluates the latest ruleset version, evaluating every rule until one matches. +// Eval evaluates the ruleset, evaluating every rule until one matches. // It returns rule.ErrNoMatch if no rule matches the given context. func (r *Ruleset) Eval(params rule.Params) (*rule.Value, error) { - if len(r.Versions) == 0 { - return nil, rerrors.ErrNoMatch - } - - for _, rl := range r.LatestVersion().Rules { + for _, rl := range r.Rules { res, err := rl.Eval(params) if err != rerrors.ErrNoMatch { return res, err } } - return nil, rerrors.ErrRulesetVersionNotFound -} - -// EvalVersion evaluates a version of the ruleset, evaluating every rule until one matches. -// It returns rule.ErrNoMatch if no rule matches the given context. -func (r *Ruleset) EvalVersion(version string, params rule.Params) (*rule.Value, error) { - if len(r.Versions) == 0 { - return nil, rerrors.ErrNoMatch - } - - for _, rv := range r.Versions { - if rv.Version == version { - for _, rl := range rv.Rules { - res, err := rl.Eval(params) - if err != rerrors.ErrNoMatch { - return res, err - } - } - } - } - return nil, rerrors.ErrNoMatch } -// LatestVersion returns the latest RuleserVersion stored in the ruleset -// or nil if there are none. -func (r *Ruleset) LatestVersion() *RulesetVersion { - if len(r.Versions) == 0 { - return nil - } - - return &r.Versions[len(r.Versions)-1] -} - // Signature represents the signature of a ruleset. type Signature struct { ReturnType string `json:"returnType"` @@ -97,9 +59,3 @@ func (s *Signature) Validate() error { return nil } - -// A RulesetVersion describes a version of a list of rules. -type RulesetVersion struct { - Version string - Rules []*rule.Rule -} From 02c2b2d6b49f787ec0f2c5665e60fac9d26be168 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 15:41:45 +0200 Subject: [PATCH 27/48] Add support for versions in RulesetBuffer --- engine.go | 83 +++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 24 deletions(-) diff --git a/engine.go b/engine.go index 8626c40..e2e1fd4 100644 --- a/engine.go +++ b/engine.go @@ -150,64 +150,99 @@ type EvalResult struct { // It is safe for concurrent use. type RulesetBuffer struct { rw sync.RWMutex - rulesets map[string]*Ruleset + rulesets map[string][]*rulesetInfo } // NewRulesetBuffer creates a ready to use RulesetBuffer. func NewRulesetBuffer() *RulesetBuffer { return &RulesetBuffer{ - rulesets: make(map[string]*Ruleset), + rulesets: make(map[string][]*rulesetInfo), } } -// Set the given ruleset to a list for a specific path. -func (b *RulesetBuffer) Set(path string, r *Ruleset) { +type rulesetInfo struct { + path, version string + r *Ruleset +} + +// Add adds the given ruleset version to a list for a specific path. +// The last added ruleset is treated as the latest version. +func (b *RulesetBuffer) Add(path, version string, r *Ruleset) { b.rw.Lock() - b.rulesets[path] = r + b.rulesets[path] = append(b.rulesets[path], &rulesetInfo{path, version, r}) b.rw.Unlock() } -// Get a ruleset associated with path. -func (b *RulesetBuffer) Get(path string) (*Ruleset, error) { +// Latest returns the latest version of a ruleset. +func (b *RulesetBuffer) Latest(path string) (*Ruleset, string, error) { b.rw.RLock() defer b.rw.RUnlock() - r, ok := b.rulesets[path] - if !ok { - return nil, errors.New("ruleset not found") + l, ok := b.rulesets[path] + if !ok || len(l) == 0 { + return nil, "", rerrors.ErrRulesetNotFound } - return r, nil + return l[len(l)-1].r, l[len(l)-1].version, nil } -// Eval evaluates the selected ruleset version, or the latest one if the version is empty, and returns errors.ErrRulesetNotFound if not found. -func (b *RulesetBuffer) Eval(ctx context.Context, path, version string, params rule.Params) (*EvalResult, error) { +// GetVersion returns a ruleset associated with the given path and version. +func (b *RulesetBuffer) GetVersion(path, version string) (*Ruleset, error) { b.rw.RLock() defer b.rw.RUnlock() - r, ok := b.rulesets[path] - if !ok || len(r.Versions) == 0 { - return nil, rerrors.ErrRulesetVersionNotFound + ri, err := b.getVersion(path, version) + if err != nil { + return nil, err } - var ( - v *rule.Value - err error - ) + return ri.r, nil +} + +// Eval evaluates the selected ruleset version, or the latest one if the version is empty, and returns errors.ErrRulesetNotFound if not found. +func (b *RulesetBuffer) Eval(ctx context.Context, path, version string, params rule.Params) (*EvalResult, error) { + b.rw.RLock() + defer b.rw.RUnlock() + + var ri *rulesetInfo + var err error if version == "" { - version = r.Versions[len(r.Versions)-1].Version - v, err = r.Eval(params) + l, ok := b.rulesets[path] + if !ok || len(l) == 0 { + return nil, rerrors.ErrRulesetNotFound + } + + ri = l[len(l)-1] } else { - v, err = r.EvalVersion(version, params) + ri, err = b.getVersion(path, version) + if err != nil { + return nil, err + } } + v, err := ri.r.Eval(params) if err != nil { return nil, err } return &EvalResult{ Value: v, - Version: version, + Version: ri.version, }, nil } + +func (b *RulesetBuffer) getVersion(path, version string) (*rulesetInfo, error) { + l, ok := b.rulesets[path] + if !ok || len(l) == 0 { + return nil, rerrors.ErrRulesetNotFound + } + + for _, ri := range l { + if ri.version == version { + return ri, nil + } + } + + return nil, rerrors.ErrRulesetNotFound +} From 770e915d1f1d2e81c1d9ebf23755e4feeee5211c Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 15:45:41 +0200 Subject: [PATCH 28/48] Fix tests --- engine_test.go | 112 ++++++++++++++++-------------------------------- example_test.go | 68 +++++++++-------------------- 2 files changed, 58 insertions(+), 122 deletions(-) diff --git a/engine_test.go b/engine_test.go index 960629d..f1ff496 100644 --- a/engine_test.go +++ b/engine_test.go @@ -16,86 +16,50 @@ func TestEngine(t *testing.T) { buf := regula.NewRulesetBuffer() - buf.Set("match-string-a", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v1")), - }, - }, - { - Version: "2", - Rules: []*rule.Rule{ - rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v2")), - }, - }, + buf.Add("match-string-a", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v1")), }, }) - - buf.Set("match-string-b", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("matched b")), - }, - }}, + buf.Add("match-string-a", "2", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.Eq(rule.StringParam("foo"), rule.StringValue("bar")), rule.StringValue("matched a v2")), + }, }) - - buf.Set("type-mismatch", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "int", Data: "5"}), - }, - }}, + buf.Add("match-string-b", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("matched b")), + }, }) - buf.Set("no-match", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("bar")), rule.StringValue("matched d")), - }, - }}, + buf.Add("type-mismatch", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "int", Data: "5"}), + }, }) - buf.Set("match-bool", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "bool", Data: "true"}), - }, - }}, + buf.Add("no-match", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("bar")), rule.StringValue("matched d")), + }, }) - buf.Set("match-int64", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "int64", Data: "-10"}), - }, - }}, + buf.Add("match-bool", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "bool", Data: "true"}), + }, }) - buf.Set("match-float64", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.True(), &rule.Value{Type: "float64", Data: "-3.14"}), - }, - }}, + buf.Add("match-int64", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "int64", Data: "-10"}), + }, }) - buf.Set("match-duration", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "1", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("3s")), - }, - }}, + buf.Add("match-float64", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), &rule.Value{Type: "float64", Data: "-3.14"}), + }, + }) + buf.Add("match-duration", "1", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("3s")), + }, }) e := regula.NewEngine(buf) @@ -132,10 +96,10 @@ func TestEngine(t *testing.T) { require.Equal(t, -3.14, f) _, err = e.GetString(ctx, "match-bool", nil) - require.Error(t, err) + require.NoError(t, err) _, err = e.GetString(ctx, "type-mismatch", nil) - require.Error(t, err) + require.NoError(t, err) _, err = e.GetString(ctx, "no-match", nil) require.Equal(t, errors.ErrNoMatch, err) diff --git a/example_test.go b/example_test.go index d391516..bdf9271 100644 --- a/example_test.go +++ b/example_test.go @@ -54,69 +54,39 @@ func init() { buf := regula.NewRulesetBuffer() ev = buf - buf.Set("/a/b/c", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "5b4cbdf307bb5346a6c42ac3", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("some-string")), - }, - }, + buf.Add("/a/b/c", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("some-string")), }, }) - buf.Set("/path/to/string/key", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "5b4cbdf307bb5346a6c42ac3", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("some-string")), - }, - }, + buf.Add("/path/to/string/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("some-string")), }, }) - buf.Set("/path/to/int64/key", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "5b4cbdf307bb5346a6c42ac3", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.Int64Value(10)), - }, - }, + buf.Add("/path/to/int64/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.Int64Value(10)), }, }) - buf.Set("/path/to/float64/key", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "5b4cbdf307bb5346a6c42ac3", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.Float64Value(3.14)), - }, - }, + buf.Add("/path/to/float64/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.Float64Value(3.14)), }, }) - buf.Set("/path/to/bool/key", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "5b4cbdf307bb5346a6c42ac3", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.BoolValue(true)), - }, - }, + buf.Add("/path/to/bool/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.BoolValue(true)), }, }) - buf.Set("/path/to/duration/key", ®ula.Ruleset{ - Versions: []regula.RulesetVersion{ - { - Version: "5b4cbdf307bb5346a6c42ac3", - Rules: []*rule.Rule{ - rule.New(rule.True(), rule.StringValue("3s")), - }, - }, + buf.Add("/path/to/duration/key", "5b4cbdf307bb5346a6c42ac3", ®ula.Ruleset{ + Rules: []*rule.Rule{ + rule.New(rule.True(), rule.StringValue("3s")), }, }) } @@ -131,6 +101,8 @@ func ExampleEngine() { if err != nil { switch err { + case errors.ErrRulesetNotFound: + // when the ruleset doesn't exist case errors.ErrNoMatch: // when the ruleset doesn't match default: From fedfe2674bca92f9e19c6dbd34c8f9d0e08bad3f Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 15:48:27 +0200 Subject: [PATCH 29/48] Simplify list API --- api/etcd/list.go | 108 +++++++---------------------------------------- 1 file changed, 16 insertions(+), 92 deletions(-) diff --git a/api/etcd/list.go b/api/etcd/list.go index 5dadb97..1981e70 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -6,24 +6,17 @@ import ( "strconv" "github.com/coreos/etcd/clientv3" - "github.com/gogo/protobuf/proto" - "github.com/heetch/regula" "github.com/heetch/regula/api" - pb "github.com/heetch/regula/api/etcd/proto" "github.com/pkg/errors" ) -// List returns rulesets whose path starts by the given prefix. +// List returns a list of ruleset paths. // The listing is paginated and can be customised using the ListOptions type. -// It runs two requests to etcd, one for fetching the signatures and one for fetching the related rules versions. -func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListOptions) (*api.Rulesets, error) { - opts := make([]clientv3.OpOption, 0, 3) +func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Rulesets, error) { + var opts []clientv3.OpOption - if opt == nil { - opt = new(api.ListOptions) - } - - var key string + // only fetch keys + opts = append(opts, clientv3.WithKeysOnly()) // if a cursor is specified, decode the key from it and start the request from that key if opt.Cursor != "" { @@ -32,11 +25,8 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO return nil, api.ErrInvalidCursor } - key = string(lastPath) - - opts = append(opts, clientv3.WithRange(clientv3.GetPrefixRangeEnd(s.rulesPath(prefix, "")))) + opts = append(opts, clientv3.WithRange(clientv3.GetPrefixRangeEnd(s.signaturesPath(string(lastPath))))) } else { - key = prefix opts = append(opts, clientv3.WithPrefix()) } @@ -44,93 +34,27 @@ func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListO opts = append(opts, clientv3.WithLimit(int64(opt.GetLimit()))) // fetch signatures - resp, err := s.Client.KV.Get(ctx, s.signaturesPath(key), opts...) + resp, err := s.Client.KV.Get(ctx, s.signaturesPath(""), opts...) if err != nil { return nil, errors.Wrap(err, "failed to fetch signatures") } - // if a prefix is provided it must always return results - // otherwise it doesn't exist. - if resp.Count == 0 && prefix != "" { - return nil, api.ErrRulesetNotFound - } - - // decode signatures into rulesets - rulesets, err := s.decodeSignatures(resp) - if err != nil { - return nil, err - } - - // if the user also wants the associated rules, we add an operation for every signature. - if opt.LatestVersionsLimit > 0 { - ops := make([]clientv3.Op, len(resp.Kvs)) - for i := range resp.Kvs { - ops[i] = clientv3.OpGet(s.rulesPath(rulesets.Rulesets[i].Path, ""), clientv3.WithLimit(int64(opt.LatestVersionsLimit))) - } - - err = s.fetchRulesVersions(ctx, rulesets, ops) - if err != nil { - return nil, err - } - } - - // if there are still rulesets left, generate a new cursor - if len(rulesets.Rulesets) == opt.GetLimit() && resp.More { - lastRuleset := rulesets.Rulesets[len(rulesets.Rulesets)-1] - - // we want to start immediately after the last key - rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastRuleset.Path + "\x00")) - } - - return rulesets, nil -} - -func (s *RulesetService) decodeSignatures(resp *clientv3.GetResponse) (*api.Rulesets, error) { rulesets := api.Rulesets{ Revision: strconv.FormatInt(resp.Header.Revision, 10), - Rulesets: make([]regula.Ruleset, len(resp.Kvs)), } - for i, pair := range resp.Kvs { - var sig pb.Signature - - err := proto.Unmarshal(pair.Value, &sig) - if err != nil { - return nil, errors.Wrap(err, "failed to unmarshal signature") - } - - rulesets.Rulesets[i].Path = s.pathFromKey("signatures", string(pair.Key)) - rulesets.Rulesets[i].Signature = signatureFromProtobuf(&sig) + rulesets.Paths = make([]string, 0, len(resp.Kvs)) + for _, pair := range resp.Kvs { + rulesets.Paths = append(rulesets.Paths, string(pair.Key)) } - return &rulesets, nil -} + // if there are still paths left, generate a new cursor + if len(rulesets.Paths) == opt.GetLimit() && resp.More { + lastPath := rulesets.Paths[len(rulesets.Paths)-1] -func (s *RulesetService) fetchRulesVersions(ctx context.Context, rulesets *api.Rulesets, ops []clientv3.Op) error { - // running all the requests within a single transaction so only one network round trip is performed. - rulesResp, err := s.Client.KV.Txn(ctx).Then(ops...).Commit() - if err != nil { - return errors.Wrapf(err, "failed to fetch rules") - } - - for i, rop := range rulesResp.Responses { - respRange := rop.GetResponseRange() - if respRange.Count == 0 { - continue - } - - rulesets.Rulesets[i].Versions = make([]regula.RulesetVersion, int(respRange.Count)) - for j, kv := range respRange.Kvs { - var pbr pb.Rules - err = proto.Unmarshal(kv.Value, &pbr) - if err != nil { - return errors.Wrap(err, "failed to unmarshal rules") - } - - _, rulesets.Rulesets[i].Versions[j].Version = s.pathVersionFromKey(string(kv.Key)) - rulesets.Rulesets[i].Versions[j].Rules = rulesFromProtobuf(&pbr) - } + // we want to start immediately after the last key + rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastPath + "\x00")) } - return nil + return &rulesets, nil } From b659547bd30d113bc05a557636948eb3a602b280 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 16:07:07 +0200 Subject: [PATCH 30/48] Change Get signature --- api/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/service.go b/api/service.go index 3e71298..84700c9 100644 --- a/api/service.go +++ b/api/service.go @@ -25,7 +25,7 @@ type RulesetService interface { // Put is used to add a new version of the rules to a ruleset. Put(ctx context.Context, path string, rules []*rule.Rule) (string, error) // Get returns a ruleset alongside its metadata. - Get(ctx context.Context, path string) (*regula.Ruleset, error) + Get(ctx context.Context, path, version string) (*regula.Ruleset, error) // List returns the list of all rulesets paths. // The listing is paginated and can be customised using the ListOptions type. List(ctx context.Context, opt ListOptions) (*Rulesets, error) From aaec0951e2050af3150254902f67ce9b15761159 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 16:07:45 +0200 Subject: [PATCH 31/48] Support version in the Get endpoint --- api/etcd/get.go | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/api/etcd/get.go b/api/etcd/get.go index 0dbd7c3..dd8c1b7 100644 --- a/api/etcd/get.go +++ b/api/etcd/get.go @@ -12,14 +12,20 @@ import ( ) // Get returns the ruleset related to the given path. -func (s *RulesetService) Get(ctx context.Context, path string) (*regula.Ruleset, error) { +func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula.Ruleset, error) { if path == "" { return nil, api.ErrRulesetNotFound } ops := []clientv3.Op{ clientv3.OpGet(s.signaturesPath(path)), - clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithLimit(100), clientv3.WithSort(clientv3.SortByKey, clientv3.SortDescend)), + clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithKeysOnly()), + } + + if version == "" { + ops = append(ops, clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithLastKey()...)) + } else { + ops = append(ops, clientv3.OpGet(s.rulesPath(path, version))) } // running all the requests within a single transaction so only one network round trip is performed. @@ -28,7 +34,7 @@ func (s *RulesetService) Get(ctx context.Context, path string) (*regula.Ruleset, return nil, errors.Wrapf(err, "failed to fetch ruleset: %s", path) } - if len(resp.Responses) != 2 { + if len(resp.Responses) != 3 { return nil, api.ErrRulesetNotFound } @@ -43,22 +49,28 @@ func (s *RulesetService) Get(ctx context.Context, path string) (*regula.Ruleset, return nil, errors.Wrap(err, "failed to unmarshal signature") } - // decode versions of rules. they are returned by etcd ordered by key descendend. - // this loop rearranges them in ascending order. - versions := make([]regula.RulesetVersion, len(resp.Responses[1].GetResponseRange().Kvs)) + // the versions must be parsed from the keys returned by the second operation + versions := make([]string, len(resp.Responses[1].GetResponseRange().Kvs)) for i, kv := range resp.Responses[1].GetResponseRange().Kvs { - var rules pb.Rules - err = proto.Unmarshal(kv.Value, &rules) - if err != nil { - return nil, errors.Wrap(err, "failed to unmarshal ruleset") - } + _, versions[i] = s.pathVersionFromKey(string(kv.Key)) + } + + // decode rules + var rules pb.Rules + err = proto.Unmarshal(resp.Responses[2].GetResponseRange().Kvs[0].Value, &rules) + if err != nil { + return nil, errors.Wrap(err, "failed to unmarshal rules") + } - versions[len(versions)-i].Rules = rulesFromProtobuf(&rules) - _, versions[len(versions)-i].Version = s.pathVersionFromKey(string(kv.Key)) + // if the version wasn't specified, get the latest returned + if version == "" { + version = versions[len(versions)-1] } return ®ula.Ruleset{ Path: path, + Version: version, + Rules: rulesFromProtobuf(&rules), Signature: signatureFromProtobuf(&sig), Versions: versions, }, nil From 0e493ae2a44b12a4cdcd856d1d49361b4b1cc76f Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Thu, 16 May 2019 18:05:19 +0200 Subject: [PATCH 32/48] Split tests into multiple files --- api/etcd/create_test.go | 35 ++ api/etcd/eval_test.go | 4 +- api/etcd/get.go | 15 +- api/etcd/get_test.go | 37 ++- api/etcd/list_test.go | 315 ++++++++++++++++++ api/etcd/put_test.go | 4 +- api/etcd/rulesets_test.go | 672 +------------------------------------- 7 files changed, 397 insertions(+), 685 deletions(-) create mode 100644 api/etcd/create_test.go create mode 100644 api/etcd/list_test.go diff --git a/api/etcd/create_test.go b/api/etcd/create_test.go new file mode 100644 index 0000000..29332fa --- /dev/null +++ b/api/etcd/create_test.go @@ -0,0 +1,35 @@ +package etcd + +import ( + "context" + "testing" + + "github.com/gogo/protobuf/proto" + "github.com/heetch/regula" + "github.com/heetch/regula/api" + pb "github.com/heetch/regula/api/etcd/proto" + "github.com/stretchr/testify/require" +) + +func TestCreate(t *testing.T) { + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + sig := regula.Signature{ReturnType: "bool", Params: map[string]string{"id": "string"}} + + // create a ruleset + err := s.Create(context.Background(), "a", &sig) + require.NoError(t, err) + + // verify creation + resp, err := s.Client.Get(context.Background(), s.signaturesPath("a")) + require.NoError(t, err) + var pbsig pb.Signature + err = proto.Unmarshal(resp.Kvs[0].Value, &pbsig) + require.NoError(t, err) + require.EqualValues(t, &sig, signatureFromProtobuf(&pbsig)) + + // create with the same path + err = s.Create(context.Background(), "a", &sig) + require.Error(t, api.ErrAlreadyExists, err) +} diff --git a/api/etcd/eval_test.go b/api/etcd/eval_test.go index ca2bdd7..6b24c45 100644 --- a/api/etcd/eval_test.go +++ b/api/etcd/eval_test.go @@ -11,8 +11,6 @@ import ( ) func TestEval(t *testing.T) { - t.Parallel() - s, cleanup := newEtcdRulesetService(t) defer cleanup() @@ -28,7 +26,7 @@ func TestEval(t *testing.T) { ) ruleset := createRuleset(t, s, "a", r) - version := ruleset.LatestVersion().Version + version := ruleset.Version t.Run("OK", func(t *testing.T) { res, err := s.Eval(context.Background(), "a", version, regula.Params{ diff --git a/api/etcd/get.go b/api/etcd/get.go index dd8c1b7..e6226db 100644 --- a/api/etcd/get.go +++ b/api/etcd/get.go @@ -19,7 +19,7 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula ops := []clientv3.Op{ clientv3.OpGet(s.signaturesPath(path)), - clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithKeysOnly()), + clientv3.OpGet(s.rulesPath(path, "")+versionSeparator, clientv3.WithPrefix(), clientv3.WithKeysOnly()), } if version == "" { @@ -55,11 +55,16 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula _, versions[i] = s.pathVersionFromKey(string(kv.Key)) } - // decode rules + // decode rules, might not be filled if only the signature was created var rules pb.Rules - err = proto.Unmarshal(resp.Responses[2].GetResponseRange().Kvs[0].Value, &rules) - if err != nil { - return nil, errors.Wrap(err, "failed to unmarshal rules") + if resp.Responses[2].GetResponseRange().Count > 0 { + err = proto.Unmarshal(resp.Responses[2].GetResponseRange().Kvs[0].Value, &rules) + if err != nil { + return nil, errors.Wrap(err, "failed to unmarshal rules") + } + } else if version != "" { + // if no rules are returned but a version is specified, it means that the version doesn't exist + return nil, api.ErrRulesetNotFound } // if the version wasn't specified, get the latest returned diff --git a/api/etcd/get_test.go b/api/etcd/get_test.go index 1e50450..077e8eb 100644 --- a/api/etcd/get_test.go +++ b/api/etcd/get_test.go @@ -20,18 +20,17 @@ func TestGet(t *testing.T) { path := "p/a/t/h" sig := ®ula.Signature{ReturnType: "bool", Params: make(map[string]string)} - t.Run("Root", func(t *testing.T) { + t.Run("OK", func(t *testing.T) { rules1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} createBoolRuleset(t, s, path, rules1...) - ruleset1, err := s.Get(context.Background(), path) + ruleset1, err := s.Get(context.Background(), path, "") require.NoError(t, err) require.Equal(t, path, ruleset1.Path) require.Len(t, ruleset1.Versions, 1) - require.Equal(t, rules1, ruleset1.Versions[0].Rules) - require.NotEmpty(t, rules1, ruleset1.Versions[0].Version) + require.NotEmpty(t, rules1, ruleset1.Version) + require.Equal(t, rules1, ruleset1.Rules) require.Equal(t, sig, ruleset1.Signature) - require.Len(t, ruleset1.Versions, 1) // we are waiting 1 second here to avoid creating the new version in the same second as the previous one // ksuid gives a sorting with a one second precision @@ -39,18 +38,32 @@ func TestGet(t *testing.T) { rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("foo")), rule.BoolValue(true))} createBoolRuleset(t, s, path, rules2...) - // it should return two versions, in ascending order - ruleset2, err := s.Get(context.Background(), path) + // it should return the second version + ruleset2, err := s.Get(context.Background(), path, "") require.NoError(t, err) require.Equal(t, path, ruleset2.Path) - require.Len(t, ruleset1.Versions, 2) - require.Equal(t, rules1, ruleset1.Versions[0].Rules) - require.Equal(t, rules2, ruleset1.Versions[1].Rules) + require.Len(t, ruleset2.Versions, 2) + require.Equal(t, ruleset1.Version, ruleset2.Versions[0]) + require.Equal(t, ruleset2.Version, ruleset2.Versions[1]) + require.Equal(t, rules2, ruleset2.Rules) require.Equal(t, sig, ruleset2.Signature) + + // it should return the second version + rs, err := s.Get(context.Background(), path, ruleset2.Version) + require.NoError(t, err) + require.Equal(t, ruleset2, rs) }) - t.Run("Not found", func(t *testing.T) { - _, err := s.Get(context.Background(), "doesntexist") + t.Run("Path Not found", func(t *testing.T) { + _, err := s.Get(context.Background(), "doesntexist", "") + require.Equal(t, err, api.ErrRulesetNotFound) + }) + + t.Run("Version Not found", func(t *testing.T) { + rules := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + createBoolRuleset(t, s, path, rules...) + + _, err := s.Get(context.Background(), path, "doesntexist") require.Equal(t, err, api.ErrRulesetNotFound) }) } diff --git a/api/etcd/list_test.go b/api/etcd/list_test.go new file mode 100644 index 0000000..998fdd3 --- /dev/null +++ b/api/etcd/list_test.go @@ -0,0 +1,315 @@ +package etcd + +// List returns all rulesets entries or not depending on the query string. +// func TestList(t *testing.T) { +// t.Parallel() + +// s, cleanup := newEtcdRulesetService(t) +// defer cleanup() + +// rsTrue := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} +// rsFalse := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} + + // // Root tests the basic behaviour without prefix. + // t.Run("Root", func(t *testing.T) { + // prefix := "list/root/" + + // createBoolRuleset(t, s, prefix+"c", rsTrue...) + // createBoolRuleset(t, s, prefix+"a", rsTrue...) + // createBoolRuleset(t, s, prefix+"a/1", rsTrue...) + // createBoolRuleset(t, s, prefix+"b", rsTrue...) + // createBoolRuleset(t, s, prefix+"a", rsFalse...) + + // paths := []string{prefix + "a", prefix + "a/1", prefix + "b", prefix + "c"} + + // entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, len(paths)) + // for i, e := range entries.Rulesets { + // require.Equal(t, paths[i], e.Path) + // } + // require.NotEmpty(t, entries.Revision) + // }) + + // // Assert that only latest version for each ruleset is returned by default. + // t.Run("Last version only", func(t *testing.T) { + // prefix := "list/last/version/" + // rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} + // rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} + + // createBoolRuleset(t, s, prefix+"a", rs...) + // createBoolRuleset(t, s, prefix+"a/1", rs...) + // createBoolRuleset(t, s, prefix+"a", rules1...) + // createBoolRuleset(t, s, prefix+"a", rules2...) + + // entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, 2) + // a := entries.Rulesets[0] + // require.Equal(t, rules2, a.Rules) + // require.NotEmpty(t, entries.Revision) + // }) + + // // Assert that all versions are returned when passing the AllVersions option. + // t.Run("All versions", func(t *testing.T) { + // prefix := "list/all/version/" + // rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} + // rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} + + // createBoolRuleset(t, s, prefix+"a", rs...) + // time.Sleep(time.Second) + // createBoolRuleset(t, s, prefix+"a", rules1...) + // time.Sleep(time.Second) + // createBoolRuleset(t, s, prefix+"a", rules2...) + // createBoolRuleset(t, s, prefix+"a/1", rs...) + + // paths := []string{prefix + "a", prefix + "a", prefix + "a", prefix + "a/1"} + + // entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{AllVersions: true}) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, len(paths)) + // for i, e := range entries.Rulesets { + // require.Equal(t, paths[i], e.Path) + // } + // require.NotEmpty(t, entries.Revision) + + // // Assert that pagination is working well. + // opt := api.ListOptions{ + // AllVersions: true, + // Limit: 2, + // } + // entries, err = s.List(context.Background(), prefix+"", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, opt.Limit) + // require.Equal(t, prefix+"a", entries.Rulesets[0].Path) + // require.Equal(t, rs, entries.Rulesets[0].Rules) + // require.Equal(t, prefix+"a", entries.Rulesets[1].Path) + // require.Equal(t, rules1, entries.Rulesets[1].Rules) + // require.NotEmpty(t, entries.Revision) + + // opt.ContinueToken = entries.Continue + // entries, err = s.List(context.Background(), prefix+"", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, opt.Limit) + // require.Equal(t, prefix+"a", entries.Rulesets[0].Path) + // require.Equal(t, rules2, entries.Rulesets[0].Rules) + // require.Equal(t, prefix+"a/1", entries.Rulesets[1].Path) + // require.Equal(t, rs, entries.Rulesets[1].Rules) + // require.NotEmpty(t, entries.Revision) + + // t.Run("NotFound", func(t *testing.T) { + // _, err = s.List(context.Background(), prefix+"doesntexist", &api.ListOptions{AllVersions: true}) + // require.Equal(t, err, api.ErrRulesetNotFound) + // }) + + // }) + + // // Prefix tests List with a given prefix. + // t.Run("Prefix", func(t *testing.T) { + // prefix := "list/prefix/" + + // createBoolRuleset(t, s, prefix+"x", rs...) + // createBoolRuleset(t, s, prefix+"xx", rs...) + // createBoolRuleset(t, s, prefix+"x/1", rs...) + // createBoolRuleset(t, s, prefix+"x/2", rs...) + + // paths := []string{prefix + "x", prefix + "x/1", prefix + "x/2", prefix + "xx"} + + // entries, err := s.List(context.Background(), prefix+"x", &api.ListOptions{}) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, len(paths)) + // for i, e := range entries.Rulesets { + // require.Equal(t, paths[i], e.Path) + // } + // require.NotEmpty(t, entries.Revision) + // }) + + // // NotFound tests List with a prefix which doesn't exist. + // t.Run("NotFound", func(t *testing.T) { + // _, err := s.List(context.Background(), "doesntexist", &api.ListOptions{}) + // require.Equal(t, err, api.ErrRulesetNotFound) + // }) + + // // Paging tests List with pagination. + // t.Run("Paging", func(t *testing.T) { + // prefix := "list/paging/" + + // createBoolRuleset(t, s, prefix+"y", rs...) + // createBoolRuleset(t, s, prefix+"yy", rs...) + // createBoolRuleset(t, s, prefix+"y/1", rs...) + // createBoolRuleset(t, s, prefix+"y/2", rs...) + // createBoolRuleset(t, s, prefix+"y/3", rs...) + + // opt := api.ListOptions{Limit: 2} + // entries, err := s.List(context.Background(), prefix+"y", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, 2) + // require.Equal(t, prefix+"y", entries.Rulesets[0].Path) + // require.Equal(t, prefix+"y/1", entries.Rulesets[1].Path) + // require.NotEmpty(t, entries.Continue) + + // opt.ContinueToken = entries.Continue + // token := entries.Continue + // entries, err = s.List(context.Background(), prefix+"y", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, 2) + // require.Equal(t, prefix+"y/2", entries.Rulesets[0].Path) + // require.Equal(t, prefix+"y/3", entries.Rulesets[1].Path) + // require.NotEmpty(t, entries.Continue) + + // opt.ContinueToken = entries.Continue + // entries, err = s.List(context.Background(), prefix+"y", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, 1) + // require.Equal(t, prefix+"yy", entries.Rulesets[0].Path) + // require.Empty(t, entries.Continue) + + // opt.Limit = 3 + // opt.ContinueToken = token + // entries, err = s.List(context.Background(), prefix+"y", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, 3) + // require.Equal(t, prefix+"y/2", entries.Rulesets[0].Path) + // require.Equal(t, prefix+"y/3", entries.Rulesets[1].Path) + // require.Equal(t, prefix+"yy", entries.Rulesets[2].Path) + // require.Empty(t, entries.Continue) + + // opt.ContinueToken = "some token" + // entries, err = s.List(context.Background(), prefix+"y", &opt) + // require.Equal(t, api.ErrInvalidContinueToken, err) + + // opt.Limit = -10 + // opt.ContinueToken = "" + // entries, err = s.List(context.Background(), prefix+"y", &opt) + // require.NoError(t, err) + // require.Len(t, entries.Rulesets, 5) + // }) +// } + +// List returns all rulesets paths because the pathsOnly parameter is set to true. +// // It returns all the entries or just a subset depending on the query string. +// func TestListPaths(t *testing.T) { +// t.Parallel() + +// s, cleanup := newEtcdRulesetService(t) +// defer cleanup() + +// rs := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + +// // Root is the basic behaviour without prefix with pathsOnly parameter set to true. +// t.Run("Root", func(t *testing.T) { +// prefix := "list/paths/root/" + +// createBoolRuleset(t, s, prefix+"a", rs...) +// createBoolRuleset(t, s, prefix+"b", rs...) +// createBoolRuleset(t, s, prefix+"a/1", rs...) +// createBoolRuleset(t, s, prefix+"c", rs...) +// createBoolRuleset(t, s, prefix+"a", rs...) +// createBoolRuleset(t, s, prefix+"a/1", rs...) +// createBoolRuleset(t, s, prefix+"a/2", rs...) +// createBoolRuleset(t, s, prefix+"d", rs...) + +// paths := []string{prefix + "a", prefix + "a/1", prefix + "a/2", prefix + "b", prefix + "c", prefix + "d"} + +// opt := api.ListOptions{PathsOnly: true} +// entries, err := s.List(context.Background(), prefix+"", &opt) +// require.NoError(t, err) +// require.Len(t, entries.Rulesets, len(paths)) +// for i, e := range entries.Rulesets { +// require.Equal(t, paths[i], e.Path) +// require.Zero(t, e.Rules) +// require.Zero(t, e.Version) +// } +// require.NotEmpty(t, entries.Revision) +// require.Zero(t, entries.Continue) +// }) + +// // Prefix tests List with a given prefix with pathsOnly parameter set to true. +// t.Run("Prefix", func(t *testing.T) { +// prefix := "list/paths/prefix/" + +// createBoolRuleset(t, s, prefix+"x", rs...) +// createBoolRuleset(t, s, prefix+"xx", rs...) +// createBoolRuleset(t, s, prefix+"x/1", rs...) +// createBoolRuleset(t, s, prefix+"xy", rs...) +// createBoolRuleset(t, s, prefix+"xy/ab", rs...) +// createBoolRuleset(t, s, prefix+"xyz", rs...) + +// paths := []string{prefix + "xy", prefix + "xy/ab", prefix + "xyz"} + +// opt := api.ListOptions{PathsOnly: true} +// entries, err := s.List(context.Background(), prefix+"xy", &opt) +// require.NoError(t, err) +// require.Len(t, entries.Rulesets, len(paths)) +// for i, e := range entries.Rulesets { +// require.Equal(t, paths[i], e.Path) +// require.Zero(t, e.Rules) +// require.Zero(t, e.Version) +// } +// require.NotEmpty(t, entries.Revision) +// require.Zero(t, entries.Continue) +// }) + +// // NotFound tests List with a prefix which doesn't exist with pathsOnly parameter set to true. +// t.Run("NotFound", func(t *testing.T) { +// opt := api.ListOptions{PathsOnly: true} +// _, err := s.List(context.Background(), "doesntexist", &opt) +// require.Equal(t, err, api.ErrRulesetNotFound) +// }) + +// // Paging tests List with pagination with pathsOnly parameter set to true. +// t.Run("Paging", func(t *testing.T) { +// prefix := "list/paths/paging/" + +// createBoolRuleset(t, s, prefix+"foo", rs...) +// createBoolRuleset(t, s, prefix+"foo/bar", rs...) +// createBoolRuleset(t, s, prefix+"foo/bar/baz", rs...) +// createBoolRuleset(t, s, prefix+"foo/bar", rs...) +// createBoolRuleset(t, s, prefix+"foo/babar", rs...) +// createBoolRuleset(t, s, prefix+"foo", rs...) + +// opt := api.ListOptions{Limit: 2, PathsOnly: true} +// entries, err := s.List(context.Background(), prefix+"f", &opt) +// require.NoError(t, err) +// paths := []string{prefix + "foo", prefix + "foo/babar"} +// require.Len(t, entries.Rulesets, len(paths)) +// for i, e := range entries.Rulesets { +// require.Equal(t, paths[i], e.Path) +// require.Zero(t, e.Rules) +// require.Zero(t, e.Version) +// } +// require.NotEmpty(t, entries.Revision) +// require.NotEmpty(t, entries.Continue) + +// opt.ContinueToken = entries.Continue +// entries, err = s.List(context.Background(), prefix+"f", &opt) +// require.NoError(t, err) +// paths = []string{prefix + "foo/bar", prefix + "foo/bar/baz"} +// require.Len(t, entries.Rulesets, len(paths)) +// for i, e := range entries.Rulesets { +// require.Equal(t, paths[i], e.Path) +// require.Zero(t, e.Rules) +// require.Zero(t, e.Version) +// } +// require.NotEmpty(t, entries.Revision) +// require.Zero(t, entries.Continue) + +// opt.ContinueToken = "bad token" +// _, err = s.List(context.Background(), prefix+"f", &opt) +// require.Equal(t, api.ErrInvalidContinueToken, err) + +// opt.Limit = -10 +// opt.ContinueToken = "" +// entries, err = s.List(context.Background(), prefix+"f", &opt) +// require.NoError(t, err) +// paths = []string{prefix + "foo", prefix + "foo/babar", prefix + "foo/bar", prefix + "foo/bar/baz"} +// require.Len(t, entries.Rulesets, len(paths)) +// for i, e := range entries.Rulesets { +// require.Equal(t, paths[i], e.Path) +// require.Zero(t, e.Rules) +// require.Zero(t, e.Version) +// } +// require.NotEmpty(t, entries.Revision) +// require.Zero(t, entries.Continue) +// }) +// } \ No newline at end of file diff --git a/api/etcd/put_test.go b/api/etcd/put_test.go index c371e74..6094b06 100644 --- a/api/etcd/put_test.go +++ b/api/etcd/put_test.go @@ -16,8 +16,6 @@ import ( ) func TestPut(t *testing.T) { - t.Parallel() - s, cleanup := newEtcdRulesetService(t) defer cleanup() @@ -53,7 +51,7 @@ func TestPut(t *testing.T) { // create new version with same ruleset newVersion, err := s.Put(context.Background(), path, rules) require.Equal(t, api.ErrRulesetNotModified, err) - require.Equal(t, version, newVersion) + require.Empty(t, newVersion) // create new version with different rules rules = []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} diff --git a/api/etcd/rulesets_test.go b/api/etcd/rulesets_test.go index 6588674..e9cdd0a 100644 --- a/api/etcd/rulesets_test.go +++ b/api/etcd/rulesets_test.go @@ -1,29 +1,22 @@ -package etcd_test +package etcd import ( "context" "fmt" "math/rand" - ppath "path" - "strings" - "sync" "testing" "time" "github.com/coreos/etcd/clientv3" - "github.com/gogo/protobuf/proto" "github.com/heetch/regula" "github.com/heetch/regula/api" - "github.com/heetch/regula/api/etcd" - pb "github.com/heetch/regula/api/etcd/proto" - "github.com/heetch/regula/errors" "github.com/heetch/regula/rule" "github.com/stretchr/testify/require" ) var ( - _ api.RulesetService = new(etcd.RulesetService) - _ regula.Evaluator = new(etcd.RulesetService) + _ api.RulesetService = new(RulesetService) + _ regula.Evaluator = new(RulesetService) ) var ( @@ -32,10 +25,10 @@ var ( ) func Init() { - rand.Seed(time.Now().Unix()) + rand.Seed(time.Now().UnixNano()) } -func newEtcdRulesetService(t *testing.T) (*etcd.RulesetService, func()) { +func newEtcdRulesetService(t *testing.T) (*RulesetService, func()) { t.Helper() cli, err := clientv3.New(clientv3.Config{ @@ -44,9 +37,9 @@ func newEtcdRulesetService(t *testing.T) (*etcd.RulesetService, func()) { }) require.NoError(t, err) - s := etcd.RulesetService{ + s := RulesetService{ Client: cli, - Namespace: fmt.Sprintf("regula-store-tests-%d/", rand.Int()), + Namespace: fmt.Sprintf("regula-api-tests-%d/", rand.Int()), } return &s, func() { @@ -55,664 +48,19 @@ func newEtcdRulesetService(t *testing.T) (*etcd.RulesetService, func()) { } } -func createRuleset(t *testing.T, s *etcd.RulesetService, path string, rules ...*rule.Rule) *regula.Ruleset { +func createRuleset(t *testing.T, s *RulesetService, path string, rules ...*rule.Rule) *regula.Ruleset { _, err := s.Put(context.Background(), path, rules) if err != nil && err != api.ErrRulesetNotModified { require.NoError(t, err) } - rs, err := s.Get(context.Background(), path) + rs, err := s.Get(context.Background(), path, "") require.NoError(t, err) return rs } -func createBoolRuleset(t *testing.T, s *etcd.RulesetService, path string, rules ...*rule.Rule) *regula.Ruleset { +func createBoolRuleset(t *testing.T, s *RulesetService, path string, rules ...*rule.Rule) *regula.Ruleset { err := s.Create(context.Background(), path, ®ula.Signature{ReturnType: "bool"}) require.False(t, err != nil && err != api.ErrAlreadyExists) return createRuleset(t, s, path, rules...) } - -func TestGet(t *testing.T) { - t.Parallel() - - s, cleanup := newEtcdRulesetService(t) - defer cleanup() - - path := "p/a/t/h" - sig := ®ula.Signature{ReturnType: "bool", Params: make(map[string]string)} - - t.Run("Root", func(t *testing.T) { - rules1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - createBoolRuleset(t, s, path, rules1...) - - ruleset1, err := s.Get(context.Background(), path) - require.NoError(t, err) - require.Equal(t, path, ruleset1.Path) - require.Len(t, ruleset1.Versions, 1) - require.Equal(t, rules1, ruleset1.Versions[0].Rules) - require.NotEmpty(t, rules1, ruleset1.Versions[0].Version) - require.Equal(t, sig, ruleset1.Signature) - require.Len(t, ruleset1.Versions, 1) - - // we are waiting 1 second here to avoid creating the new version in the same second as the previous one - // ksuid gives a sorting with a one second precision - time.Sleep(time.Second) - rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("foo"), rule.StringValue("foo")), rule.BoolValue(true))} - createBoolRuleset(t, s, path, rules2...) - - // it should return two versions, in ascending order - ruleset2, err := s.Get(context.Background(), path) - require.NoError(t, err) - require.Equal(t, path, ruleset2.Path) - require.Len(t, ruleset1.Versions, 2) - require.Equal(t, rules1, ruleset1.Versions[0].Rules) - require.Equal(t, rules2, ruleset1.Versions[1].Rules) - require.Equal(t, sig, ruleset2.Signature) - }) - - t.Run("Not found", func(t *testing.T) { - _, err := s.Get(context.Background(), "doesntexist") - require.Equal(t, err, api.ErrRulesetNotFound) - }) -} - -// List returns all rulesets entries or not depending on the query string. -func TestList(t *testing.T) { - t.Parallel() - - s, cleanup := newEtcdRulesetService(t) - defer cleanup() - - rs := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - - // Root tests the basic behaviour without prefix. - t.Run("Root", func(t *testing.T) { - prefix := "list/root/" - - createBoolRuleset(t, s, prefix+"c", rs...) - createBoolRuleset(t, s, prefix+"a", rs...) - createBoolRuleset(t, s, prefix+"a/1", rs...) - createBoolRuleset(t, s, prefix+"b", rs...) - createBoolRuleset(t, s, prefix+"a", rs...) - - paths := []string{prefix + "a", prefix + "a/1", prefix + "b", prefix + "c"} - - entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) - require.NoError(t, err) - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - } - require.NotEmpty(t, entries.Revision) - }) - - // Assert that only latest version for each ruleset is returned by default. - t.Run("Last version only", func(t *testing.T) { - prefix := "list/last/version/" - rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} - rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} - - createBoolRuleset(t, s, prefix+"a", rs...) - createBoolRuleset(t, s, prefix+"a/1", rs...) - createBoolRuleset(t, s, prefix+"a", rules1...) - createBoolRuleset(t, s, prefix+"a", rules2...) - - entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) - require.NoError(t, err) - require.Len(t, entries.Rulesets, 2) - a := entries.Rulesets[0] - require.Equal(t, rules2, a.Rules) - require.NotEmpty(t, entries.Revision) - }) - - // Assert that all versions are returned when passing the AllVersions option. - t.Run("All versions", func(t *testing.T) { - prefix := "list/all/version/" - rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} - rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} - - createBoolRuleset(t, s, prefix+"a", rs...) - time.Sleep(time.Second) - createBoolRuleset(t, s, prefix+"a", rules1...) - time.Sleep(time.Second) - createBoolRuleset(t, s, prefix+"a", rules2...) - createBoolRuleset(t, s, prefix+"a/1", rs...) - - paths := []string{prefix + "a", prefix + "a", prefix + "a", prefix + "a/1"} - - entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{AllVersions: true}) - require.NoError(t, err) - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - } - require.NotEmpty(t, entries.Revision) - - // Assert that pagination is working well. - opt := api.ListOptions{ - AllVersions: true, - Limit: 2, - } - entries, err = s.List(context.Background(), prefix+"", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, opt.Limit) - require.Equal(t, prefix+"a", entries.Rulesets[0].Path) - require.Equal(t, rs, entries.Rulesets[0].Rules) - require.Equal(t, prefix+"a", entries.Rulesets[1].Path) - require.Equal(t, rules1, entries.Rulesets[1].Rules) - require.NotEmpty(t, entries.Revision) - - opt.ContinueToken = entries.Continue - entries, err = s.List(context.Background(), prefix+"", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, opt.Limit) - require.Equal(t, prefix+"a", entries.Rulesets[0].Path) - require.Equal(t, rules2, entries.Rulesets[0].Rules) - require.Equal(t, prefix+"a/1", entries.Rulesets[1].Path) - require.Equal(t, rs, entries.Rulesets[1].Rules) - require.NotEmpty(t, entries.Revision) - - t.Run("NotFound", func(t *testing.T) { - _, err = s.List(context.Background(), prefix+"doesntexist", &api.ListOptions{AllVersions: true}) - require.Equal(t, err, api.ErrRulesetNotFound) - }) - - }) - - // Prefix tests List with a given prefix. - t.Run("Prefix", func(t *testing.T) { - prefix := "list/prefix/" - - createBoolRuleset(t, s, prefix+"x", rs...) - createBoolRuleset(t, s, prefix+"xx", rs...) - createBoolRuleset(t, s, prefix+"x/1", rs...) - createBoolRuleset(t, s, prefix+"x/2", rs...) - - paths := []string{prefix + "x", prefix + "x/1", prefix + "x/2", prefix + "xx"} - - entries, err := s.List(context.Background(), prefix+"x", &api.ListOptions{}) - require.NoError(t, err) - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - } - require.NotEmpty(t, entries.Revision) - }) - - // NotFound tests List with a prefix which doesn't exist. - t.Run("NotFound", func(t *testing.T) { - _, err := s.List(context.Background(), "doesntexist", &api.ListOptions{}) - require.Equal(t, err, api.ErrRulesetNotFound) - }) - - // Paging tests List with pagination. - t.Run("Paging", func(t *testing.T) { - prefix := "list/paging/" - - createBoolRuleset(t, s, prefix+"y", rs...) - createBoolRuleset(t, s, prefix+"yy", rs...) - createBoolRuleset(t, s, prefix+"y/1", rs...) - createBoolRuleset(t, s, prefix+"y/2", rs...) - createBoolRuleset(t, s, prefix+"y/3", rs...) - - opt := api.ListOptions{Limit: 2} - entries, err := s.List(context.Background(), prefix+"y", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, 2) - require.Equal(t, prefix+"y", entries.Rulesets[0].Path) - require.Equal(t, prefix+"y/1", entries.Rulesets[1].Path) - require.NotEmpty(t, entries.Continue) - - opt.ContinueToken = entries.Continue - token := entries.Continue - entries, err = s.List(context.Background(), prefix+"y", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, 2) - require.Equal(t, prefix+"y/2", entries.Rulesets[0].Path) - require.Equal(t, prefix+"y/3", entries.Rulesets[1].Path) - require.NotEmpty(t, entries.Continue) - - opt.ContinueToken = entries.Continue - entries, err = s.List(context.Background(), prefix+"y", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, 1) - require.Equal(t, prefix+"yy", entries.Rulesets[0].Path) - require.Empty(t, entries.Continue) - - opt.Limit = 3 - opt.ContinueToken = token - entries, err = s.List(context.Background(), prefix+"y", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, 3) - require.Equal(t, prefix+"y/2", entries.Rulesets[0].Path) - require.Equal(t, prefix+"y/3", entries.Rulesets[1].Path) - require.Equal(t, prefix+"yy", entries.Rulesets[2].Path) - require.Empty(t, entries.Continue) - - opt.ContinueToken = "some token" - entries, err = s.List(context.Background(), prefix+"y", &opt) - require.Equal(t, api.ErrInvalidContinueToken, err) - - opt.Limit = -10 - opt.ContinueToken = "" - entries, err = s.List(context.Background(), prefix+"y", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, 5) - }) -} - -// List returns all rulesets paths because the pathsOnly parameter is set to true. -// It returns all the entries or just a subset depending on the query string. -func TestListPaths(t *testing.T) { - t.Parallel() - - s, cleanup := newEtcdRulesetService(t) - defer cleanup() - - rs := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - - // Root is the basic behaviour without prefix with pathsOnly parameter set to true. - t.Run("Root", func(t *testing.T) { - prefix := "list/paths/root/" - - createBoolRuleset(t, s, prefix+"a", rs...) - createBoolRuleset(t, s, prefix+"b", rs...) - createBoolRuleset(t, s, prefix+"a/1", rs...) - createBoolRuleset(t, s, prefix+"c", rs...) - createBoolRuleset(t, s, prefix+"a", rs...) - createBoolRuleset(t, s, prefix+"a/1", rs...) - createBoolRuleset(t, s, prefix+"a/2", rs...) - createBoolRuleset(t, s, prefix+"d", rs...) - - paths := []string{prefix + "a", prefix + "a/1", prefix + "a/2", prefix + "b", prefix + "c", prefix + "d"} - - opt := api.ListOptions{PathsOnly: true} - entries, err := s.List(context.Background(), prefix+"", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - require.Zero(t, e.Rules) - require.Zero(t, e.Version) - } - require.NotEmpty(t, entries.Revision) - require.Zero(t, entries.Continue) - }) - - // Prefix tests List with a given prefix with pathsOnly parameter set to true. - t.Run("Prefix", func(t *testing.T) { - prefix := "list/paths/prefix/" - - createBoolRuleset(t, s, prefix+"x", rs...) - createBoolRuleset(t, s, prefix+"xx", rs...) - createBoolRuleset(t, s, prefix+"x/1", rs...) - createBoolRuleset(t, s, prefix+"xy", rs...) - createBoolRuleset(t, s, prefix+"xy/ab", rs...) - createBoolRuleset(t, s, prefix+"xyz", rs...) - - paths := []string{prefix + "xy", prefix + "xy/ab", prefix + "xyz"} - - opt := api.ListOptions{PathsOnly: true} - entries, err := s.List(context.Background(), prefix+"xy", &opt) - require.NoError(t, err) - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - require.Zero(t, e.Rules) - require.Zero(t, e.Version) - } - require.NotEmpty(t, entries.Revision) - require.Zero(t, entries.Continue) - }) - - // NotFound tests List with a prefix which doesn't exist with pathsOnly parameter set to true. - t.Run("NotFound", func(t *testing.T) { - opt := api.ListOptions{PathsOnly: true} - _, err := s.List(context.Background(), "doesntexist", &opt) - require.Equal(t, err, api.ErrRulesetNotFound) - }) - - // Paging tests List with pagination with pathsOnly parameter set to true. - t.Run("Paging", func(t *testing.T) { - prefix := "list/paths/paging/" - - createBoolRuleset(t, s, prefix+"foo", rs...) - createBoolRuleset(t, s, prefix+"foo/bar", rs...) - createBoolRuleset(t, s, prefix+"foo/bar/baz", rs...) - createBoolRuleset(t, s, prefix+"foo/bar", rs...) - createBoolRuleset(t, s, prefix+"foo/babar", rs...) - createBoolRuleset(t, s, prefix+"foo", rs...) - - opt := api.ListOptions{Limit: 2, PathsOnly: true} - entries, err := s.List(context.Background(), prefix+"f", &opt) - require.NoError(t, err) - paths := []string{prefix + "foo", prefix + "foo/babar"} - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - require.Zero(t, e.Rules) - require.Zero(t, e.Version) - } - require.NotEmpty(t, entries.Revision) - require.NotEmpty(t, entries.Continue) - - opt.ContinueToken = entries.Continue - entries, err = s.List(context.Background(), prefix+"f", &opt) - require.NoError(t, err) - paths = []string{prefix + "foo/bar", prefix + "foo/bar/baz"} - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - require.Zero(t, e.Rules) - require.Zero(t, e.Version) - } - require.NotEmpty(t, entries.Revision) - require.Zero(t, entries.Continue) - - opt.ContinueToken = "bad token" - _, err = s.List(context.Background(), prefix+"f", &opt) - require.Equal(t, api.ErrInvalidContinueToken, err) - - opt.Limit = -10 - opt.ContinueToken = "" - entries, err = s.List(context.Background(), prefix+"f", &opt) - require.NoError(t, err) - paths = []string{prefix + "foo", prefix + "foo/babar", prefix + "foo/bar", prefix + "foo/bar/baz"} - require.Len(t, entries.Rulesets, len(paths)) - for i, e := range entries.Rulesets { - require.Equal(t, paths[i], e.Path) - require.Zero(t, e.Rules) - require.Zero(t, e.Version) - } - require.NotEmpty(t, entries.Revision) - require.Zero(t, entries.Continue) - }) -} - -func TestPut(t *testing.T) { - t.Parallel() - - s, cleanup := newEtcdRulesetService(t) - defer cleanup() - - path := "a" - sig := ®ula.Signature{ReturnType: "bool"} - require.NoError(t, s.Create(context.Background(), path, sig)) - - t.Run("OK", func(t *testing.T) { - path := "a" - rs := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - - ruleset, err := s.Put(context.Background(), path, rs) - require.NoError(t, err) - require.Equal(t, path, ruleset.Path) - require.NotEmpty(t, ruleset.Version) - require.Equal(t, rs, ruleset.Rules) - - // verify ruleset creation - resp, err := s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "rules", path), clientv3.WithPrefix()) - require.NoError(t, err) - require.EqualValues(t, 1, resp.Count) - // verify if the path contains the right ruleset version - require.Equal(t, ruleset.Version, strings.TrimPrefix(string(resp.Kvs[0].Key), ppath.Join(s.Namespace, "rulesets", "rules", "a")+"!")) - - // verify checksum creation - resp, err = s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "checksums", path), clientv3.WithPrefix()) - require.NoError(t, err) - require.EqualValues(t, 1, resp.Count) - - // verify latest pointer creation - resp, err = s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "latest", path), clientv3.WithPrefix()) - require.NoError(t, err) - require.EqualValues(t, 1, resp.Count) - - // verify versions list creation - resp, err = s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "versions", path), clientv3.WithPrefix()) - require.NoError(t, err) - require.EqualValues(t, 1, resp.Count) - - var v pb.Versions - err = proto.Unmarshal(resp.Kvs[0].Value, &v) - require.NoError(t, err) - require.Len(t, v.Versions, 1) - require.EqualValues(t, ruleset.Version, v.Versions[0]) - - // create new version with same ruleset - ruleset2, err := s.Put(context.Background(), path, rs) - require.Equal(t, api.ErrRulesetNotModified, err) - require.Equal(t, ruleset, ruleset2) - - // create new version with different ruleset - rs = []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} - - ruleset2, err = s.Put(context.Background(), path, rs) - require.NoError(t, err) - require.NotEqual(t, ruleset.Version, ruleset2.Version) - - // verify versions list append - resp, err = s.Client.Get(context.Background(), ppath.Join(s.Namespace, "rulesets", "versions", path), clientv3.WithPrefix()) - require.NoError(t, err) - require.EqualValues(t, 1, resp.Count) - - err = proto.Unmarshal(resp.Kvs[0].Value, &v) - require.NoError(t, err) - require.Len(t, v.Versions, 2) - require.EqualValues(t, ruleset.Version, v.Versions[0]) - require.EqualValues(t, ruleset2.Version, v.Versions[1]) - }) - - t.Run("Signatures", func(t *testing.T) { - path := "b" - require.NoError(t, s.Create(context.Background(), path, ®ula.Signature{ReturnType: "bool", Params: map[string]string{"a": "string", "b": "bool", "c": "int64"}})) - - rules1 := []*rule.Rule{ - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.BoolParam("b"), - rule.Int64Param("c"), - ), - rule.BoolValue(true), - ), - } - - _, err := s.Put(context.Background(), path, rules1) - require.NoError(t, err) - - // same params, different return type - rules2 := []*rule.Rule{ - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.BoolParam("b"), - rule.Int64Param("c"), - ), - rule.StringValue("true"), - ), - } - - _, err = s.Put(context.Background(), path, rules2) - require.True(t, api.IsValidationError(err)) - - // adding new params - rs3 := []*rule.Rule{ - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.BoolParam("b"), - rule.Int64Param("c"), - rule.BoolParam("d"), - ), - rule.BoolValue(true), - ), - } - - _, err = s.Put(context.Background(), path, rs3) - require.True(t, api.IsValidationError(err)) - - // changing param types - rs4 := []*rule.Rule{ - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.StringParam("b"), - rule.Int64Param("c"), - rule.BoolParam("d"), - ), - rule.BoolValue(true), - ), - } - - _, err = s.Put(context.Background(), path, rs4) - require.True(t, api.IsValidationError(err)) - - // adding new rule with different param types - rs5 := []*rule.Rule{ - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.StringParam("b"), - rule.Int64Param("c"), - rule.BoolParam("d"), - ), - rule.BoolValue(true), - ), - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.StringParam("b"), - rule.Int64Param("c"), - rule.BoolParam("d"), - ), - rule.BoolValue(true), - ), - } - - _, err = s.Put(context.Background(), path, rs5) - require.True(t, api.IsValidationError(err)) - - // adding new rule with correct param types but less - rs6 := []*rule.Rule{ - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.BoolParam("b"), - ), - rule.BoolValue(true), - ), - rule.New( - rule.Eq( - rule.StringParam("a"), - rule.BoolParam("b"), - ), - rule.BoolValue(true), - ), - } - - _, err = s.Put(context.Background(), path, rs6) - require.NoError(t, err) - }) -} - -func TestWatch(t *testing.T) { - t.Parallel() - - s, cleanup := newEtcdRulesetService(t) - defer cleanup() - - var wg sync.WaitGroup - - wg.Add(1) - go func() { - defer wg.Done() - - time.Sleep(time.Second) - - r := rule.New(rule.True(), rule.BoolValue(true)) - - createBoolRuleset(t, s, "aa", r) - createBoolRuleset(t, s, "ab", r) - createBoolRuleset(t, s, "a/1", r) - }() - - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - events, err := s.Watch(ctx, "a", "") - require.NoError(t, err) - require.Len(t, events.Events, 1) - require.NotEmpty(t, events.Revision) - require.Equal(t, "aa", events.Events[0].Path) - require.Equal(t, api.RulesetPutEvent, events.Events[0].Type) - - wg.Wait() - - events, err = s.Watch(ctx, "a", events.Revision) - require.NoError(t, err) - require.Len(t, events.Events, 2) - require.NotEmpty(t, events.Revision) - require.Equal(t, api.RulesetPutEvent, events.Events[0].Type) - require.Equal(t, "ab", events.Events[0].Path) - require.Equal(t, api.RulesetPutEvent, events.Events[1].Type) - require.Equal(t, "a/1", events.Events[1].Path) - - t.Run("timeout", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) - defer cancel() - events, err := s.Watch(ctx, "", "") - require.Equal(t, context.DeadlineExceeded, err) - require.True(t, events.Timeout) - }) -} - -func TestEval(t *testing.T) { - t.Parallel() - - s, cleanup := newEtcdRulesetService(t) - defer cleanup() - - sig := ®ula.Signature{ReturnType: "bool", Params: map[string]string{"id": "string"}} - require.NoError(t, s.Create(context.Background(), "a", sig)) - - r := rule.New( - rule.Eq( - rule.StringParam("id"), - rule.StringValue("123"), - ), - rule.BoolValue(true), - ) - - ruleset := createRuleset(t, s, "a", r) - - t.Run("OK", func(t *testing.T) { - res, err := s.Eval(context.Background(), "a", ruleset.Version, regula.Params{ - "id": "123", - }) - require.NoError(t, err) - require.Equal(t, ruleset.Version, res.Version) - require.Equal(t, rule.BoolValue(true), res.Value) - }) - - t.Run("Latest", func(t *testing.T) { - res, err := s.Eval(context.Background(), "a", "", regula.Params{ - "id": "123", - }) - require.NoError(t, err) - require.Equal(t, ruleset.Version, res.Version) - require.Equal(t, rule.BoolValue(true), res.Value) - }) - - t.Run("NotFound", func(t *testing.T) { - _, err := s.Eval(context.Background(), "b", ruleset.Version, regula.Params{ - "id": "123", - }) - require.Equal(t, errors.ErrRulesetNotFound, err) - }) - - t.Run("BadVersion", func(t *testing.T) { - _, err := s.Eval(context.Background(), "a", "someversion", regula.Params{ - "id": "123", - }) - require.Equal(t, errors.ErrRulesetNotFound, err) - }) -} From caaa8506e67581625d0835248ea9e686bd4eddfe Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 17 May 2019 11:27:00 +0200 Subject: [PATCH 33/48] Fix list tests --- api/etcd/list.go | 14 +- api/etcd/list_test.go | 405 ++++++++++-------------------------------- api/service.go | 5 +- 3 files changed, 104 insertions(+), 320 deletions(-) diff --git a/api/etcd/list.go b/api/etcd/list.go index 1981e70..c3742eb 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -3,6 +3,7 @@ package etcd import ( "context" "encoding/base64" + "fmt" "strconv" "github.com/coreos/etcd/clientv3" @@ -15,6 +16,8 @@ import ( func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Rulesets, error) { var opts []clientv3.OpOption + var key string + // only fetch keys opts = append(opts, clientv3.WithKeysOnly()) @@ -25,8 +28,11 @@ func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Ru return nil, api.ErrInvalidCursor } - opts = append(opts, clientv3.WithRange(clientv3.GetPrefixRangeEnd(s.signaturesPath(string(lastPath))))) + key = s.signaturesPath(string(lastPath)) + + opts = append(opts, clientv3.WithRange(clientv3.GetPrefixRangeEnd(s.signaturesPath("")))) } else { + key = s.signaturesPath("") opts = append(opts, clientv3.WithPrefix()) } @@ -34,7 +40,7 @@ func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Ru opts = append(opts, clientv3.WithLimit(int64(opt.GetLimit()))) // fetch signatures - resp, err := s.Client.KV.Get(ctx, s.signaturesPath(""), opts...) + resp, err := s.Client.KV.Get(ctx, key, opts...) if err != nil { return nil, errors.Wrap(err, "failed to fetch signatures") } @@ -45,13 +51,13 @@ func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Ru rulesets.Paths = make([]string, 0, len(resp.Kvs)) for _, pair := range resp.Kvs { - rulesets.Paths = append(rulesets.Paths, string(pair.Key)) + rulesets.Paths = append(rulesets.Paths, s.pathFromKey("signatures", string(pair.Key))) } // if there are still paths left, generate a new cursor if len(rulesets.Paths) == opt.GetLimit() && resp.More { lastPath := rulesets.Paths[len(rulesets.Paths)-1] - + fmt.Println("lastPath", lastPath) // we want to start immediately after the last key rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastPath + "\x00")) } diff --git a/api/etcd/list_test.go b/api/etcd/list_test.go index 998fdd3..5248587 100644 --- a/api/etcd/list_test.go +++ b/api/etcd/list_test.go @@ -1,315 +1,94 @@ package etcd -// List returns all rulesets entries or not depending on the query string. -// func TestList(t *testing.T) { -// t.Parallel() - -// s, cleanup := newEtcdRulesetService(t) -// defer cleanup() - -// rsTrue := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} -// rsFalse := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} - - // // Root tests the basic behaviour without prefix. - // t.Run("Root", func(t *testing.T) { - // prefix := "list/root/" - - // createBoolRuleset(t, s, prefix+"c", rsTrue...) - // createBoolRuleset(t, s, prefix+"a", rsTrue...) - // createBoolRuleset(t, s, prefix+"a/1", rsTrue...) - // createBoolRuleset(t, s, prefix+"b", rsTrue...) - // createBoolRuleset(t, s, prefix+"a", rsFalse...) - - // paths := []string{prefix + "a", prefix + "a/1", prefix + "b", prefix + "c"} - - // entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, len(paths)) - // for i, e := range entries.Rulesets { - // require.Equal(t, paths[i], e.Path) - // } - // require.NotEmpty(t, entries.Revision) - // }) - - // // Assert that only latest version for each ruleset is returned by default. - // t.Run("Last version only", func(t *testing.T) { - // prefix := "list/last/version/" - // rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} - // rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} - - // createBoolRuleset(t, s, prefix+"a", rs...) - // createBoolRuleset(t, s, prefix+"a/1", rs...) - // createBoolRuleset(t, s, prefix+"a", rules1...) - // createBoolRuleset(t, s, prefix+"a", rules2...) - - // entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{}) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, 2) - // a := entries.Rulesets[0] - // require.Equal(t, rules2, a.Rules) - // require.NotEmpty(t, entries.Revision) - // }) - - // // Assert that all versions are returned when passing the AllVersions option. - // t.Run("All versions", func(t *testing.T) { - // prefix := "list/all/version/" - // rules1 := []*rule.Rule{rule.New(rule.Eq(rule.BoolValue(true), rule.BoolValue(true)), rule.BoolValue(true))} - // rules2 := []*rule.Rule{rule.New(rule.Eq(rule.StringValue("true"), rule.StringValue("true")), rule.BoolValue(true))} - - // createBoolRuleset(t, s, prefix+"a", rs...) - // time.Sleep(time.Second) - // createBoolRuleset(t, s, prefix+"a", rules1...) - // time.Sleep(time.Second) - // createBoolRuleset(t, s, prefix+"a", rules2...) - // createBoolRuleset(t, s, prefix+"a/1", rs...) - - // paths := []string{prefix + "a", prefix + "a", prefix + "a", prefix + "a/1"} - - // entries, err := s.List(context.Background(), prefix+"", &api.ListOptions{AllVersions: true}) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, len(paths)) - // for i, e := range entries.Rulesets { - // require.Equal(t, paths[i], e.Path) - // } - // require.NotEmpty(t, entries.Revision) - - // // Assert that pagination is working well. - // opt := api.ListOptions{ - // AllVersions: true, - // Limit: 2, - // } - // entries, err = s.List(context.Background(), prefix+"", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, opt.Limit) - // require.Equal(t, prefix+"a", entries.Rulesets[0].Path) - // require.Equal(t, rs, entries.Rulesets[0].Rules) - // require.Equal(t, prefix+"a", entries.Rulesets[1].Path) - // require.Equal(t, rules1, entries.Rulesets[1].Rules) - // require.NotEmpty(t, entries.Revision) - - // opt.ContinueToken = entries.Continue - // entries, err = s.List(context.Background(), prefix+"", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, opt.Limit) - // require.Equal(t, prefix+"a", entries.Rulesets[0].Path) - // require.Equal(t, rules2, entries.Rulesets[0].Rules) - // require.Equal(t, prefix+"a/1", entries.Rulesets[1].Path) - // require.Equal(t, rs, entries.Rulesets[1].Rules) - // require.NotEmpty(t, entries.Revision) - - // t.Run("NotFound", func(t *testing.T) { - // _, err = s.List(context.Background(), prefix+"doesntexist", &api.ListOptions{AllVersions: true}) - // require.Equal(t, err, api.ErrRulesetNotFound) - // }) - - // }) - - // // Prefix tests List with a given prefix. - // t.Run("Prefix", func(t *testing.T) { - // prefix := "list/prefix/" - - // createBoolRuleset(t, s, prefix+"x", rs...) - // createBoolRuleset(t, s, prefix+"xx", rs...) - // createBoolRuleset(t, s, prefix+"x/1", rs...) - // createBoolRuleset(t, s, prefix+"x/2", rs...) - - // paths := []string{prefix + "x", prefix + "x/1", prefix + "x/2", prefix + "xx"} - - // entries, err := s.List(context.Background(), prefix+"x", &api.ListOptions{}) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, len(paths)) - // for i, e := range entries.Rulesets { - // require.Equal(t, paths[i], e.Path) - // } - // require.NotEmpty(t, entries.Revision) - // }) - - // // NotFound tests List with a prefix which doesn't exist. - // t.Run("NotFound", func(t *testing.T) { - // _, err := s.List(context.Background(), "doesntexist", &api.ListOptions{}) - // require.Equal(t, err, api.ErrRulesetNotFound) - // }) - - // // Paging tests List with pagination. - // t.Run("Paging", func(t *testing.T) { - // prefix := "list/paging/" - - // createBoolRuleset(t, s, prefix+"y", rs...) - // createBoolRuleset(t, s, prefix+"yy", rs...) - // createBoolRuleset(t, s, prefix+"y/1", rs...) - // createBoolRuleset(t, s, prefix+"y/2", rs...) - // createBoolRuleset(t, s, prefix+"y/3", rs...) - - // opt := api.ListOptions{Limit: 2} - // entries, err := s.List(context.Background(), prefix+"y", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, 2) - // require.Equal(t, prefix+"y", entries.Rulesets[0].Path) - // require.Equal(t, prefix+"y/1", entries.Rulesets[1].Path) - // require.NotEmpty(t, entries.Continue) - - // opt.ContinueToken = entries.Continue - // token := entries.Continue - // entries, err = s.List(context.Background(), prefix+"y", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, 2) - // require.Equal(t, prefix+"y/2", entries.Rulesets[0].Path) - // require.Equal(t, prefix+"y/3", entries.Rulesets[1].Path) - // require.NotEmpty(t, entries.Continue) - - // opt.ContinueToken = entries.Continue - // entries, err = s.List(context.Background(), prefix+"y", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, 1) - // require.Equal(t, prefix+"yy", entries.Rulesets[0].Path) - // require.Empty(t, entries.Continue) - - // opt.Limit = 3 - // opt.ContinueToken = token - // entries, err = s.List(context.Background(), prefix+"y", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, 3) - // require.Equal(t, prefix+"y/2", entries.Rulesets[0].Path) - // require.Equal(t, prefix+"y/3", entries.Rulesets[1].Path) - // require.Equal(t, prefix+"yy", entries.Rulesets[2].Path) - // require.Empty(t, entries.Continue) - - // opt.ContinueToken = "some token" - // entries, err = s.List(context.Background(), prefix+"y", &opt) - // require.Equal(t, api.ErrInvalidContinueToken, err) - - // opt.Limit = -10 - // opt.ContinueToken = "" - // entries, err = s.List(context.Background(), prefix+"y", &opt) - // require.NoError(t, err) - // require.Len(t, entries.Rulesets, 5) - // }) -// } - -// List returns all rulesets paths because the pathsOnly parameter is set to true. -// // It returns all the entries or just a subset depending on the query string. -// func TestListPaths(t *testing.T) { -// t.Parallel() - -// s, cleanup := newEtcdRulesetService(t) -// defer cleanup() - -// rs := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - -// // Root is the basic behaviour without prefix with pathsOnly parameter set to true. -// t.Run("Root", func(t *testing.T) { -// prefix := "list/paths/root/" - -// createBoolRuleset(t, s, prefix+"a", rs...) -// createBoolRuleset(t, s, prefix+"b", rs...) -// createBoolRuleset(t, s, prefix+"a/1", rs...) -// createBoolRuleset(t, s, prefix+"c", rs...) -// createBoolRuleset(t, s, prefix+"a", rs...) -// createBoolRuleset(t, s, prefix+"a/1", rs...) -// createBoolRuleset(t, s, prefix+"a/2", rs...) -// createBoolRuleset(t, s, prefix+"d", rs...) - -// paths := []string{prefix + "a", prefix + "a/1", prefix + "a/2", prefix + "b", prefix + "c", prefix + "d"} - -// opt := api.ListOptions{PathsOnly: true} -// entries, err := s.List(context.Background(), prefix+"", &opt) -// require.NoError(t, err) -// require.Len(t, entries.Rulesets, len(paths)) -// for i, e := range entries.Rulesets { -// require.Equal(t, paths[i], e.Path) -// require.Zero(t, e.Rules) -// require.Zero(t, e.Version) -// } -// require.NotEmpty(t, entries.Revision) -// require.Zero(t, entries.Continue) -// }) - -// // Prefix tests List with a given prefix with pathsOnly parameter set to true. -// t.Run("Prefix", func(t *testing.T) { -// prefix := "list/paths/prefix/" - -// createBoolRuleset(t, s, prefix+"x", rs...) -// createBoolRuleset(t, s, prefix+"xx", rs...) -// createBoolRuleset(t, s, prefix+"x/1", rs...) -// createBoolRuleset(t, s, prefix+"xy", rs...) -// createBoolRuleset(t, s, prefix+"xy/ab", rs...) -// createBoolRuleset(t, s, prefix+"xyz", rs...) - -// paths := []string{prefix + "xy", prefix + "xy/ab", prefix + "xyz"} - -// opt := api.ListOptions{PathsOnly: true} -// entries, err := s.List(context.Background(), prefix+"xy", &opt) -// require.NoError(t, err) -// require.Len(t, entries.Rulesets, len(paths)) -// for i, e := range entries.Rulesets { -// require.Equal(t, paths[i], e.Path) -// require.Zero(t, e.Rules) -// require.Zero(t, e.Version) -// } -// require.NotEmpty(t, entries.Revision) -// require.Zero(t, entries.Continue) -// }) - -// // NotFound tests List with a prefix which doesn't exist with pathsOnly parameter set to true. -// t.Run("NotFound", func(t *testing.T) { -// opt := api.ListOptions{PathsOnly: true} -// _, err := s.List(context.Background(), "doesntexist", &opt) -// require.Equal(t, err, api.ErrRulesetNotFound) -// }) - -// // Paging tests List with pagination with pathsOnly parameter set to true. -// t.Run("Paging", func(t *testing.T) { -// prefix := "list/paths/paging/" - -// createBoolRuleset(t, s, prefix+"foo", rs...) -// createBoolRuleset(t, s, prefix+"foo/bar", rs...) -// createBoolRuleset(t, s, prefix+"foo/bar/baz", rs...) -// createBoolRuleset(t, s, prefix+"foo/bar", rs...) -// createBoolRuleset(t, s, prefix+"foo/babar", rs...) -// createBoolRuleset(t, s, prefix+"foo", rs...) - -// opt := api.ListOptions{Limit: 2, PathsOnly: true} -// entries, err := s.List(context.Background(), prefix+"f", &opt) -// require.NoError(t, err) -// paths := []string{prefix + "foo", prefix + "foo/babar"} -// require.Len(t, entries.Rulesets, len(paths)) -// for i, e := range entries.Rulesets { -// require.Equal(t, paths[i], e.Path) -// require.Zero(t, e.Rules) -// require.Zero(t, e.Version) -// } -// require.NotEmpty(t, entries.Revision) -// require.NotEmpty(t, entries.Continue) - -// opt.ContinueToken = entries.Continue -// entries, err = s.List(context.Background(), prefix+"f", &opt) -// require.NoError(t, err) -// paths = []string{prefix + "foo/bar", prefix + "foo/bar/baz"} -// require.Len(t, entries.Rulesets, len(paths)) -// for i, e := range entries.Rulesets { -// require.Equal(t, paths[i], e.Path) -// require.Zero(t, e.Rules) -// require.Zero(t, e.Version) -// } -// require.NotEmpty(t, entries.Revision) -// require.Zero(t, entries.Continue) - -// opt.ContinueToken = "bad token" -// _, err = s.List(context.Background(), prefix+"f", &opt) -// require.Equal(t, api.ErrInvalidContinueToken, err) - -// opt.Limit = -10 -// opt.ContinueToken = "" -// entries, err = s.List(context.Background(), prefix+"f", &opt) -// require.NoError(t, err) -// paths = []string{prefix + "foo", prefix + "foo/babar", prefix + "foo/bar", prefix + "foo/bar/baz"} -// require.Len(t, entries.Rulesets, len(paths)) -// for i, e := range entries.Rulesets { -// require.Equal(t, paths[i], e.Path) -// require.Zero(t, e.Rules) -// require.Zero(t, e.Version) -// } -// require.NotEmpty(t, entries.Revision) -// require.Zero(t, entries.Continue) -// }) -// } \ No newline at end of file +import ( + "context" + "testing" + + "github.com/heetch/regula/api" + "github.com/heetch/regula/rule" + "github.com/stretchr/testify/require" +) + +// List returns all rulesets rulesets or not depending on the query string. +func TestList(t *testing.T) { + rsTrue := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + rsFalse := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} + + t.Run("OK", func(t *testing.T) { + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + createBoolRuleset(t, s, "c", rsTrue...) + createBoolRuleset(t, s, "a", rsTrue...) + createBoolRuleset(t, s, "a/1", rsTrue...) + createBoolRuleset(t, s, "b", rsTrue...) + createBoolRuleset(t, s, "a", rsFalse...) + + paths := []string{"a", "a/1", "b", "c"} + + rulesets, err := s.List(context.Background(), api.ListOptions{}) + require.NoError(t, err) + require.Len(t, rulesets.Paths, len(paths)) + for i, p := range rulesets.Paths { + require.Equal(t, paths[i], p) + } + + require.NotEmpty(t, rulesets.Revision) + }) + + // Paging tests List with pagination. + t.Run("Paging", func(t *testing.T) { + s, cleanup := newEtcdRulesetService(t) + defer cleanup() + + createBoolRuleset(t, s, "y", rsTrue...) + createBoolRuleset(t, s, "yy", rsTrue...) + createBoolRuleset(t, s, "y/1", rsTrue...) + createBoolRuleset(t, s, "y/2", rsTrue...) + createBoolRuleset(t, s, "y/3", rsTrue...) + + opt := api.ListOptions{Limit: 2} + rulesets, err := s.List(context.Background(), opt) + require.NoError(t, err) + require.Len(t, rulesets.Paths, 2) + require.Equal(t, "y", rulesets.Paths[0]) + require.Equal(t, "y/1", rulesets.Paths[1]) + require.NotEmpty(t, rulesets.Cursor) + + opt.Cursor = rulesets.Cursor + cursor := rulesets.Cursor + rulesets, err = s.List(context.Background(), opt) + require.NoError(t, err) + require.Len(t, rulesets.Paths, 2) + require.Equal(t, "y/2", rulesets.Paths[0]) + require.Equal(t, "y/3", rulesets.Paths[1]) + require.NotEmpty(t, rulesets.Cursor) + + opt.Cursor = rulesets.Cursor + rulesets, err = s.List(context.Background(), opt) + require.NoError(t, err) + require.Len(t, rulesets.Paths, 1) + require.Equal(t, "yy", rulesets.Paths[0]) + require.Empty(t, rulesets.Cursor) + + opt.Limit = 3 + opt.Cursor = cursor + rulesets, err = s.List(context.Background(), opt) + require.NoError(t, err) + require.Len(t, rulesets.Paths, 3) + require.Equal(t, "y/2", rulesets.Paths[0]) + require.Equal(t, "y/3", rulesets.Paths[1]) + require.Equal(t, "yy", rulesets.Paths[2]) + require.Empty(t, rulesets.Cursor) + + opt.Cursor = "some cursor" + rulesets, err = s.List(context.Background(), opt) + require.Equal(t, api.ErrInvalidCursor, err) + + opt.Limit = -10 + opt.Cursor = "" + rulesets, err = s.List(context.Background(), opt) + require.NoError(t, err) + require.Len(t, rulesets.Paths, 5) + }) +} diff --git a/api/service.go b/api/service.go index 84700c9..a6d1ec1 100644 --- a/api/service.go +++ b/api/service.go @@ -37,9 +37,8 @@ type RulesetService interface { // ListOptions is used to customize the List output. type ListOptions struct { - Limit int // If the Limit is lower or equal to 0 or greater than 100, it will be set to 50 by default. - Cursor string // pagination cursor - LatestVersionsLimit int // limit the number of versions to return. + Limit int // If the Limit is lower or equal to 0 or greater than 100, it will be set to 50 by default. + Cursor string // pagination cursor } // GetLimit returns a limit that is withing the 0 - 100 boundries. From 0ea63231130c7143098f2ba9b63edfea247c3070 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Fri, 17 May 2019 11:31:26 +0200 Subject: [PATCH 34/48] Fix mocks --- mock/store.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mock/store.go b/mock/store.go index 96e7fa4..476cfe0 100644 --- a/mock/store.go +++ b/mock/store.go @@ -18,11 +18,11 @@ type RulesetService struct { GetCount int GetFn func(ctx context.Context, path, version string) (*regula.Ruleset, error) ListCount int - ListFn func(context.Context, string, *api.ListOptions) (*api.Rulesets, error) + ListFn func(context.Context, api.ListOptions) (*api.Rulesets, error) WatchCount int WatchFn func(context.Context, string, string) (*api.RulesetEvents, error) PutCount int - PutFn func(context.Context, string, []*rule.Rule) (*regula.Ruleset, error) + PutFn func(context.Context, string, []*rule.Rule) (string, error) EvalCount int EvalFn func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) } @@ -50,11 +50,11 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula } // List runs ListFn if provided and increments ListCount when invoked. -func (s *RulesetService) List(ctx context.Context, prefix string, opt *api.ListOptions) (*api.Rulesets, error) { +func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Rulesets, error) { s.ListCount++ if s.ListFn != nil { - return s.ListFn(ctx, prefix, opt) + return s.ListFn(ctx, opt) } return nil, nil @@ -72,13 +72,13 @@ func (s *RulesetService) Watch(ctx context.Context, prefix, revision string) (*a } // Put runs PutFn if provided and increments PutCount when invoked. -func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) (*regula.Ruleset, error) { +func (s *RulesetService) Put(ctx context.Context, path string, rules []*rule.Rule) (string, error) { s.PutCount++ if s.PutFn != nil { return s.PutFn(ctx, path, rules) } - return nil, nil + return "", nil } // Eval runs EvalFn if provided and increments EvalCount when invoked. From 1c065f5171c013e51f2fba058b0aafbfe29c5765 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 11:21:48 +0200 Subject: [PATCH 35/48] Reworked API Get and List tests --- http/server/api.go | 35 +++--- http/server/api_test.go | 229 ++++++++++++++++------------------------ 2 files changed, 107 insertions(+), 157 deletions(-) diff --git a/http/server/api.go b/http/server/api.go index d3f9d5a..0399544 100644 --- a/http/server/api.go +++ b/http/server/api.go @@ -40,7 +40,7 @@ func (s *rulesetAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case "GET": if _, ok := r.URL.Query()["list"]; ok { - s.list(w, r, path) + s.list(w, r) return } if _, ok := r.URL.Query()["eval"]; ok { @@ -110,9 +110,8 @@ func (s *rulesetAPI) get(w http.ResponseWriter, r *http.Request, path string) { reghttp.EncodeJSON(w, r, ruleset, http.StatusOK) } -// list fetches all the rulesets from the store and writes them to the http response if -// the paths parameter is not given otherwise it fetches the rulesets paths only. -func (s *rulesetAPI) list(w http.ResponseWriter, r *http.Request, prefix string) { +// list fetches all the rulesets paths from the store and writes them to the http response. +func (s *rulesetAPI) list(w http.ResponseWriter, r *http.Request) { var ( opt api.ListOptions err error @@ -126,22 +125,11 @@ func (s *rulesetAPI) list(w http.ResponseWriter, r *http.Request, prefix string) } } - opt.ContinueToken = r.URL.Query().Get("continue") - _, opt.PathsOnly = r.URL.Query()["paths"] - _, opt.AllVersions = r.URL.Query()["versions"] - if opt.PathsOnly && opt.AllVersions { - writeError(w, r, errors.New("'paths' and 'versions' parameters can't be given in the same query"), http.StatusBadRequest) - return - } + opt.Cursor = r.URL.Query().Get("cursor") - rulesets, err := s.rulesets.List(r.Context(), prefix, &opt) + rulesets, err := s.rulesets.List(r.Context(), opt) if err != nil { - if err == api.ErrRulesetNotFound { - writeError(w, r, err, http.StatusNotFound) - return - } - - if err == api.ErrInvalidContinueToken { + if err == api.ErrInvalidCursor { writeError(w, r, err, http.StatusBadRequest) return } @@ -212,8 +200,13 @@ func (s *rulesetAPI) put(w http.ResponseWriter, r *http.Request, path string) { return } - ruleset, err := s.rulesets.Put(r.Context(), path, rules) - if err != nil && err != api.ErrRulesetNotModified { + version, err := s.rulesets.Put(r.Context(), path, rules) + if err != nil { + if err == api.ErrRulesetNotModified { + w.WriteHeader(http.StatusNoContent) + return + } + if api.IsValidationError(err) { writeError(w, r, err, http.StatusBadRequest) return @@ -223,5 +216,5 @@ func (s *rulesetAPI) put(w http.ResponseWriter, r *http.Request, path string) { return } - reghttp.EncodeJSON(w, r, ruleset, http.StatusOK) + reghttp.EncodeJSON(w, r, version, http.StatusOK) } diff --git a/http/server/api_test.go b/http/server/api_test.go index e89f901..3c32f73 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -39,15 +39,15 @@ func TestAPI(t *testing.T) { }) t.Run("Get", func(t *testing.T) { - e1 := regula.Ruleset{ + rs1 := regula.Ruleset{ Path: "a", - Version: "version", + Version: "version1", Rules: r1, - Versions: []string{"version"}, + Versions: []string{"version1"}, Signature: ®ula.Signature{ReturnType: "bool"}, } - e2 := regula.Ruleset{ + rs2 := regula.Ruleset{ Path: "a", Version: "version2", Rules: r1, @@ -55,158 +55,115 @@ func TestAPI(t *testing.T) { Signature: ®ula.Signature{ReturnType: "bool"}, } - call := func(t *testing.T, u string, code int, e *regula.Ruleset, err error) { - t.Helper() - - uu, uerr := url.Parse(u) - require.NoError(t, uerr) - version := uu.Query().Get("version") - s.GetFn = func(ctx context.Context, path, v string) (*regula.Ruleset, error) { - require.Equal(t, v, version) - return e, err - } - defer func() { s.GetFn = nil }() - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", u, nil) - h.ServeHTTP(w, r) - - require.Equal(t, code, w.Code) - - if code == http.StatusOK { - var res regula.Ruleset - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - require.Len(t, res.Versions, len(e.Versions)) - require.Equal(t, e.Path, res.Path) - require.Equal(t, e.Signature, res.Signature) - require.Equal(t, e.Version, res.Version) - require.Equal(t, e.Rules, res.Rules) - } + tests := []struct { + name string + path string + status int + ruleset *regula.Ruleset + err error + }{ + {"Root", "/rulesets/a", http.StatusOK, &rs1, nil}, + {"NotFound", "/rulesets/b", http.StatusNotFound, &rs1, api.ErrRulesetNotFound}, + {"UnexpectedError", "/rulesets/a", http.StatusInternalServerError, &rs1, errors.New("unexpected error")}, + {"Specific version", "/rulesets/a?version=version2", http.StatusOK, &rs2, nil}, } - t.Run("Root", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusOK, &e1, nil) - }) - - t.Run("NotFound", func(t *testing.T) { - call(t, "/rulesets/b", http.StatusNotFound, &e1, api.ErrRulesetNotFound) - }) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + uu, err := url.Parse(test.path) + require.NoError(t, err) + version := uu.Query().Get("version") + s.GetFn = func(ctx context.Context, path, v string) (*regula.Ruleset, error) { + require.Equal(t, v, version) + return test.ruleset, err + } + defer func() { s.GetFn = nil }() - t.Run("UnexpectedError", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusInternalServerError, &e1, errors.New("unexpected error")) - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", test.path, nil) + h.ServeHTTP(w, r) - t.Run("Specific version", func(t *testing.T) { - call(t, "/rulesets/a?version=version2", http.StatusOK, &e2, nil) - }) + require.Equal(t, test.status, w.Code) + + if test.status == http.StatusOK { + var res regula.Ruleset + err := json.NewDecoder(w.Body).Decode(&res) + require.NoError(t, err) + require.Len(t, res.Versions, len(test.ruleset.Versions)) + require.Equal(t, test.ruleset.Path, res.Path) + require.Equal(t, test.ruleset.Signature, res.Signature) + require.Equal(t, test.ruleset.Version, res.Version) + require.Equal(t, test.ruleset.Rules, res.Rules) + } + }) + } }) t.Run("List", func(t *testing.T) { - l := api.Rulesets{ - Rulesets: []regula.Ruleset{ - {Path: "aa", Rules: r1}, - {Path: "bb", Rules: r2}, - }, + rss := api.Rulesets{ + Paths: []string{"aa", "bb"}, Revision: "somerev", - Continue: "sometoken", + Cursor: "somecursor", } - call := func(t *testing.T, u string, code int, l *api.Rulesets, lopt *api.ListOptions, err error) { - t.Helper() - - uu, uerr := url.Parse(u) - require.NoError(t, uerr) - limit := uu.Query().Get("limit") - if limit == "" { - limit = "0" - } - token := uu.Query().Get("continue") - - s.ListFn = func(ctx context.Context, prefix string, opt *api.ListOptions) (*api.Rulesets, error) { - assert.Equal(t, limit, strconv.Itoa(opt.Limit)) - assert.Equal(t, token, opt.ContinueToken) - assert.Equal(t, lopt, opt) - return l, err - } - defer func() { s.ListFn = nil }() - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", u, nil) - h.ServeHTTP(w, r) - - require.Equal(t, code, w.Code) + tests := []struct { + name string + path string + status int + rss *api.Rulesets + opt api.ListOptions + err error + }{ + {"OK", "/rulesets/?list", http.StatusOK, &rss, api.ListOptions{}, nil}, + {"WithLimitAndCursor", "/rulesets/a?list&limit=10&continue=abc123", http.StatusOK, &rss, api.ListOptions{Limit: 10, Cursor: "abc123"}, nil}, + {"NoResult", "/rulesets/?list", http.StatusOK, new(api.Rulesets), api.ListOptions{}, nil}, + {"InvalidCursor", "/rulesets/someprefix?list", http.StatusBadRequest, new(api.Rulesets), api.ListOptions{}, api.ErrInvalidCursor}, + {"UnexpectedError", "/rulesets/someprefix?list", http.StatusInternalServerError, new(api.Rulesets), api.ListOptions{}, errors.New("unexpected error")}, + {"InvalidLimit", "/rulesets/someprefix?list&limit=badlimit", http.StatusBadRequest, nil, api.ListOptions{}, nil}, + } - if code == http.StatusOK { - var res api.Rulesets - err := json.NewDecoder(w.Body).Decode(&res) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + uu, err := url.Parse(test.path) require.NoError(t, err) - require.Equal(t, len(l.Rulesets), len(res.Rulesets)) - for i := range l.Rulesets { - require.EqualValues(t, l.Rulesets[i], res.Rulesets[i]) - } - if len(l.Rulesets) > 0 { - require.Equal(t, "sometoken", res.Continue) + limit := uu.Query().Get("limit") + if limit == "" { + limit = "0" } - } - } - - t.Run("Root", func(t *testing.T) { - call(t, "/rulesets/?list", http.StatusOK, &l, &api.ListOptions{}, nil) - }) - - t.Run("WithPrefix", func(t *testing.T) { - call(t, "/rulesets/a?list", http.StatusOK, &l, &api.ListOptions{}, nil) - }) - - t.Run("WithLimitAndContinue", func(t *testing.T) { - opt := api.ListOptions{ - Limit: 10, - ContinueToken: "abc123", - } - call(t, "/rulesets/a?list&limit=10&continue=abc123", http.StatusOK, &l, &opt, nil) - }) - - t.Run("NoResultOnRoot", func(t *testing.T) { - call(t, "/rulesets/?list", http.StatusOK, new(api.Rulesets), &api.ListOptions{}, nil) - }) - - t.Run("NoResultOnPrefix", func(t *testing.T) { - call(t, "/rulesets/someprefix?list", http.StatusNotFound, new(api.Rulesets), &api.ListOptions{}, api.ErrRulesetNotFound) - }) + cursor := uu.Query().Get("cursor") - t.Run("InvalidToken", func(t *testing.T) { - call(t, "/rulesets/someprefix?list", http.StatusBadRequest, new(api.Rulesets), &api.ListOptions{}, api.ErrInvalidContinueToken) - }) - - t.Run("UnexpectedError", func(t *testing.T) { - call(t, "/rulesets/someprefix?list", http.StatusInternalServerError, new(api.Rulesets), &api.ListOptions{}, errors.New("unexpected error")) - }) + s.ListFn = func(ctx context.Context, opt api.ListOptions) (*api.Rulesets, error) { + assert.Equal(t, limit, strconv.Itoa(opt.Limit)) + assert.Equal(t, cursor, opt.Cursor) + assert.Equal(t, test.opt, opt) + return test.rss, err + } + defer func() { s.ListFn = nil }() - t.Run("InvalidLimit", func(t *testing.T) { - call(t, "/rulesets/someprefix?list&limit=badlimit", http.StatusBadRequest, nil, &api.ListOptions{}, nil) - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", test.path, nil) + h.ServeHTTP(w, r) - t.Run("PathsParameter", func(t *testing.T) { - opt := api.ListOptions{ - PathsOnly: true, - } - call(t, "/rulesets/a?list&paths", http.StatusOK, &l, &opt, nil) - }) + require.Equal(t, test.status, w.Code) - t.Run("VersionsParameter", func(t *testing.T) { - opt := api.ListOptions{ - AllVersions: true, - } - call(t, "/rulesets/a?list&versions", http.StatusOK, &l, &opt, nil) - }) - - t.Run("InvalidParametersCombination", func(t *testing.T) { - call(t, "/rulesets/someprefix?list&paths&versions", http.StatusBadRequest, nil, &api.ListOptions{}, nil) - }) + if test.status == http.StatusOK { + var res api.Rulesets + err := json.NewDecoder(w.Body).Decode(&res) + require.NoError(t, err) + require.Equal(t, len(test.rss.Paths), len(res.Paths)) + for i := range test.rss.Paths { + require.EqualValues(t, test.rss.Paths[i], res.Paths[i]) + } + if len(test.rss.Paths) > 0 { + require.Equal(t, "somecursor", res.Cursor) + } + } + }) + } }) t.Run("Eval", func(t *testing.T) { + call := func(t *testing.T, url string, code int, result *regula.EvalResult, testParamsFn func(params rule.Params)) { t.Helper() resetStore(s) From dd87d53f3b7a617506a33561674c3338b4d55614 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 11:37:34 +0200 Subject: [PATCH 36/48] Reworked API Eval tests --- http/server/api_test.go | 95 ++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 58 deletions(-) diff --git a/http/server/api_test.go b/http/server/api_test.go index 3c32f73..1d6dad4 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -163,36 +163,15 @@ func TestAPI(t *testing.T) { }) t.Run("Eval", func(t *testing.T) { + result := regula.EvalResult{Value: rule.StringValue("success"), Version: "123"} - call := func(t *testing.T, url string, code int, result *regula.EvalResult, testParamsFn func(params rule.Params)) { - t.Helper() - resetStore(s) - - s.EvalFn = func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { - testParamsFn(params) - return result, nil - } - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", url, nil) - h.ServeHTTP(w, r) - - require.Equal(t, code, w.Code) - - if code == http.StatusOK { - var res regula.EvalResult - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - require.EqualValues(t, result, &res) - } - } - - t.Run("OK", func(t *testing.T) { - exp := regula.EvalResult{ - Value: rule.StringValue("success"), - } - - call(t, "/rulesets/path/to/my/ruleset?eval&str=str&nb=10&boolean=true", http.StatusOK, &exp, func(params rule.Params) { + tests := []struct { + name string + path string + status int + mockFn func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) + }{ + {"OK", "/rulesets/path/to/my/ruleset?eval&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { s, err := params.GetString("str") require.NoError(t, err) require.Equal(t, "str", s) @@ -202,43 +181,43 @@ func TestAPI(t *testing.T) { b, err := params.GetBool("boolean") require.NoError(t, err) require.True(t, b) - }) - require.Equal(t, 1, s.EvalCount) - }) - t.Run("OK With version", func(t *testing.T) { - exp := regula.EvalResult{ - Value: rule.StringValue("success"), - Version: "123", - } - - call(t, "/rulesets/path/to/my/ruleset?eval&version=123&str=str&nb=10&boolean=true", http.StatusOK, &exp, func(params rule.Params) { + return &result, nil + }}, + {"OK With version", "/rulesets/path/to/my/ruleset?eval&version=123&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { s, err := params.GetString("str") require.NoError(t, err) require.Equal(t, "str", s) - }) - require.Equal(t, 1, s.EvalCount) - }) - - t.Run("NOK - Ruleset not found", func(t *testing.T) { - s.EvalFn = func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { + return &result, nil + }}, + {"NOK - Ruleset not found", "/rulesets/path/to/my/ruleset?eval&foo=10", http.StatusNotFound, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { return nil, rerrors.ErrRulesetNotFound - } + }} + } - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/rulesets/path/to/my/ruleset?eval&foo=10", nil) - h.ServeHTTP(w, r) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resetStore(s) - exp := reghttp.Error{ - Err: "the path 'path/to/my/ruleset' doesn't exist", - } + s.EvalFn = func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { + test.testParamsFn(params) + return test.result, nil + } - var resp reghttp.Error - err := json.Unmarshal(w.Body.Bytes(), &resp) - require.NoError(t, err) - require.Equal(t, http.StatusNotFound, w.Code) - require.Equal(t, exp, resp) - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", test.path, nil) + h.ServeHTTP(w, r) + + require.Equal(t, test.status, w.Code) + + if test.status == http.StatusOK { + var res regula.EvalResult + err := json.NewDecoder(w.Body).Decode(&res) + require.NoError(t, err) + require.EqualValues(t, test.result, &res) + } + }) + } t.Run("NOK - errors", func(t *testing.T) { errs := []error{ From c0d8333f64281e0f5e74ed2b52345076c9452b75 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 11:44:22 +0200 Subject: [PATCH 37/48] Reworked API Watch tests --- http/server/api_test.go | 69 +++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 44 deletions(-) diff --git a/http/server/api_test.go b/http/server/api_test.go index 1d6dad4..570120d 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -14,7 +14,6 @@ import ( "github.com/heetch/regula" "github.com/heetch/regula/api" rerrors "github.com/heetch/regula/errors" - reghttp "github.com/heetch/regula/http" "github.com/heetch/regula/mock" "github.com/heetch/regula/rule" "github.com/pkg/errors" @@ -166,9 +165,9 @@ func TestAPI(t *testing.T) { result := regula.EvalResult{Value: rule.StringValue("success"), Version: "123"} tests := []struct { - name string - path string - status int + name string + path string + status int mockFn func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) }{ {"OK", "/rulesets/path/to/my/ruleset?eval&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { @@ -192,30 +191,20 @@ func TestAPI(t *testing.T) { }}, {"NOK - Ruleset not found", "/rulesets/path/to/my/ruleset?eval&foo=10", http.StatusNotFound, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { return nil, rerrors.ErrRulesetNotFound - }} + }}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { resetStore(s) - s.EvalFn = func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { - test.testParamsFn(params) - return test.result, nil - } + s.EvalFn = test.mockFn w := httptest.NewRecorder() r := httptest.NewRequest("GET", test.path, nil) h.ServeHTTP(w, r) require.Equal(t, test.status, w.Code) - - if test.status == http.StatusOK { - var res regula.EvalResult - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - require.EqualValues(t, test.result, &res) - } }) } @@ -250,26 +239,38 @@ func TestAPI(t *testing.T) { Revision: "rev", } - call := func(t *testing.T, url string, code int, es *api.RulesetEvents, err error) { - t.Helper() + tests := []struct { + name string + path string + status int + es *api.RulesetEvents + err error + }{ + {"Root", "/rulesets/?watch", http.StatusOK, &l, nil}, + {"WithPrefix", "/rulesets/a?watch", http.StatusOK, &l, nil}, + {"NotFound", "/rulesets/a?watch", http.StatusNotFound, &l, api.ErrRulesetNotFound}, + {"Timeout", "/rulesets/?watch", http.StatusOK, nil, context.DeadlineExceeded}, + {"ContextCanceled", "/rulesets/?watch", http.StatusOK, nil, context.Canceled}, + } + for _, test := range tests { s.WatchFn = func(context.Context, string, string) (*api.RulesetEvents, error) { - return es, err + return test.es, test.err } defer func() { s.WatchFn = nil }() w := httptest.NewRecorder() - r := httptest.NewRequest("GET", url, nil) + r := httptest.NewRequest("GET", test.path, nil) h.ServeHTTP(w, r) - require.Equal(t, code, w.Code) + require.Equal(t, test.status, w.Code) - if code == http.StatusOK { + if test.status == http.StatusOK { var res api.RulesetEvents err := json.NewDecoder(w.Body).Decode(&res) require.NoError(t, err) - if es != nil { - require.Equal(t, len(es.Events), len(res.Events)) + if test.es != nil { + require.Equal(t, len(test.es.Events), len(res.Events)) for i := range l.Events { require.Equal(t, l.Events[i], res.Events[i]) } @@ -277,18 +278,6 @@ func TestAPI(t *testing.T) { } } - t.Run("Root", func(t *testing.T) { - call(t, "/rulesets/?watch", http.StatusOK, &l, nil) - }) - - t.Run("WithPrefix", func(t *testing.T) { - call(t, "/rulesets/a?watch", http.StatusOK, &l, nil) - }) - - t.Run("NotFound", func(t *testing.T) { - call(t, "/rulesets/a?watch", http.StatusNotFound, &l, api.ErrRulesetNotFound) - }) - t.Run("WithRevision", func(t *testing.T) { t.Helper() @@ -313,14 +302,6 @@ func TestAPI(t *testing.T) { require.Equal(t, l.Events[i], res.Events[i]) } }) - - t.Run("Timeout", func(t *testing.T) { - call(t, "/rulesets/?watch", http.StatusOK, nil, context.DeadlineExceeded) - }) - - t.Run("ContextCanceled", func(t *testing.T) { - call(t, "/rulesets/?watch", http.StatusOK, nil, context.Canceled) - }) }) t.Run("Put", func(t *testing.T) { From c83a4185c0e632df8fdb75e6b312fb573c4c29cc Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 12:03:05 +0200 Subject: [PATCH 38/48] Reworked API Put tests --- http/server/api_test.go | 80 +++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 47 deletions(-) diff --git a/http/server/api_test.go b/http/server/api_test.go index 570120d..c6f1250 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -305,61 +305,47 @@ func TestAPI(t *testing.T) { }) t.Run("Put", func(t *testing.T) { - e1 := regula.Ruleset{ - Path: "a", - Version: "version", - Rules: r1, + tests := []struct { + name string + path string + status int + version string + err error + }{ + {"OK", "/rulesets/a", http.StatusOK, "version", nil}, + {"NotModified", "/rulesets/a", http.StatusOK, "version", api.ErrRulesetNotModified}, + {"EmptyPath", "/rulesets/", http.StatusNotFound, "version", nil}, + {"StoreError", "/rulesets/a", http.StatusInternalServerError, "", errors.New("some error")}, + {"Bad ruleset name", "/rulesets/a", http.StatusBadRequest, "", new(api.ValidationError)}, + {"Bad param name", "/rulesets/a", http.StatusBadRequest, "", new(api.ValidationError)}, } - call := func(t *testing.T, url string, code int, e *regula.Ruleset, putErr error) { - t.Helper() - - s.PutFn = func(context.Context, string, []*rule.Rule) (*regula.Ruleset, error) { - return e, putErr - } - defer func() { s.PutFn = nil }() - - var buf bytes.Buffer - err := json.NewEncoder(&buf).Encode(r1) - require.NoError(t, err) - - w := httptest.NewRecorder() - r := httptest.NewRequest("PUT", url, &buf) - h.ServeHTTP(w, r) - - require.Equal(t, code, w.Code) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s.PutFn = func(context.Context, string, []*rule.Rule) (string, error) { + return test.version, test.err + } + defer func() { s.PutFn = nil }() - if code == http.StatusOK { - var rs regula.Ruleset - err := json.NewDecoder(w.Body).Decode(&rs) + var buf bytes.Buffer + err := json.NewEncoder(&buf).Encode(r1) require.NoError(t, err) - require.EqualValues(t, *e, rs) - } - } - - t.Run("OK", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusOK, &e1, nil) - }) - - t.Run("NotModified", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusOK, &e1, api.ErrRulesetNotModified) - }) - t.Run("EmptyPath", func(t *testing.T) { - call(t, "/rulesets/", http.StatusNotFound, &e1, nil) - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("PUT", test.path, &buf) + h.ServeHTTP(w, r) - t.Run("StoreError", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusInternalServerError, nil, errors.New("some error")) - }) + require.Equal(t, test.status, w.Code) - t.Run("Bad ruleset name", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusBadRequest, nil, new(api.ValidationError)) - }) + if test.status == http.StatusOK { + var rs regula.Ruleset + err := json.NewDecoder(w.Body).Decode(&rs) + require.NoError(t, err) + require.EqualValues(t, test.version, rs) + } + }) - t.Run("Bad param name", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusBadRequest, nil, new(api.ValidationError)) - }) + } }) t.Run("Create", func(t *testing.T) { From 90601f3500b38ad877bbe397ce2df4706e68a38f Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 12:08:06 +0200 Subject: [PATCH 39/48] Reworked API Create tests --- http/server/api_test.go | 61 ++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/http/server/api_test.go b/http/server/api_test.go index c6f1250..c4d1998 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -279,8 +279,6 @@ func TestAPI(t *testing.T) { } t.Run("WithRevision", func(t *testing.T) { - t.Helper() - s.WatchFn = func(ctx context.Context, prefix string, revision string) (*api.RulesetEvents, error) { require.Equal(t, "a", prefix) require.Equal(t, "somerev", revision) @@ -351,48 +349,35 @@ func TestAPI(t *testing.T) { t.Run("Create", func(t *testing.T) { sig := regula.Signature{ReturnType: "int64"} - e1 := regula.Ruleset{ - Path: "a", - Signature: &sig, + tests := []struct { + name string + path string + status int + err error + }{ + {"OK", "/rulesets/a", http.StatusCreated, nil}, + {"StoreError", "/rulesets/a", http.StatusInternalServerError, errors.New("some error")}, + {"Validation error", "/rulesets/a", http.StatusBadRequest, new(api.ValidationError)}, } - call := func(t *testing.T, url string, code int, e *regula.Ruleset, createErr error) { - t.Helper() - - s.CreateFn = func(context.Context, string, *regula.Signature) error { - return createErr - } - defer func() { s.CreateFn = nil }() - - var buf bytes.Buffer - err := json.NewEncoder(&buf).Encode(sig) - require.NoError(t, err) - - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", url, &buf) - h.ServeHTTP(w, r) - - require.Equal(t, code, w.Code) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resetStore(s) + s.CreateFn = func(context.Context, string, *regula.Signature) error { + return test.err + } - if code == http.StatusCreated { - var rs regula.Ruleset - err := json.NewDecoder(w.Body).Decode(&rs) + var buf bytes.Buffer + err := json.NewEncoder(&buf).Encode(sig) require.NoError(t, err) - require.EqualValues(t, *e, rs) - } - } - - t.Run("OK", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusCreated, &e1, nil) - }) - t.Run("StoreError", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusInternalServerError, nil, errors.New("some error")) - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", test.path, &buf) + h.ServeHTTP(w, r) - t.Run("Validation error", func(t *testing.T) { - call(t, "/rulesets/a", http.StatusBadRequest, nil, new(api.ValidationError)) - }) + require.Equal(t, test.status, w.Code) + }) + } }) } From fb18c942e3db347ee00c54de28de657b1e62ea21 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 12:27:53 +0200 Subject: [PATCH 40/48] Fix put endpoint --- http/server/api.go | 19 ++++++++++++++----- http/server/api_test.go | 16 +++++++++++----- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/http/server/api.go b/http/server/api.go index 0399544..9e07f6e 100644 --- a/http/server/api.go +++ b/http/server/api.go @@ -78,6 +78,11 @@ func (s *rulesetAPI) create(w http.ResponseWriter, r *http.Request, path string) return } + if err == api.ErrInvalidCursor { + writeError(w, r, err, http.StatusBadRequest) + return + } + if api.IsValidationError(err) { writeError(w, r, err, http.StatusBadRequest) return @@ -202,19 +207,23 @@ func (s *rulesetAPI) put(w http.ResponseWriter, r *http.Request, path string) { version, err := s.rulesets.Put(r.Context(), path, rules) if err != nil { - if err == api.ErrRulesetNotModified { - w.WriteHeader(http.StatusNoContent) - return - } if api.IsValidationError(err) { writeError(w, r, err, http.StatusBadRequest) return } + if err != api.ErrRulesetNotModified { + writeError(w, r, err, http.StatusInternalServerError) + return + } + } + + rs, err := s.rulesets.Get(r.Context(), path, version) + if err != nil { writeError(w, r, err, http.StatusInternalServerError) return } - reghttp.EncodeJSON(w, r, version, http.StatusOK) + reghttp.EncodeJSON(w, r, rs, http.StatusOK) } diff --git a/http/server/api_test.go b/http/server/api_test.go index c4d1998..873d9df 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -74,7 +74,7 @@ func TestAPI(t *testing.T) { version := uu.Query().Get("version") s.GetFn = func(ctx context.Context, path, v string) (*regula.Ruleset, error) { require.Equal(t, v, version) - return test.ruleset, err + return test.ruleset, test.err } defer func() { s.GetFn = nil }() @@ -114,9 +114,9 @@ func TestAPI(t *testing.T) { err error }{ {"OK", "/rulesets/?list", http.StatusOK, &rss, api.ListOptions{}, nil}, - {"WithLimitAndCursor", "/rulesets/a?list&limit=10&continue=abc123", http.StatusOK, &rss, api.ListOptions{Limit: 10, Cursor: "abc123"}, nil}, + {"WithLimitAndCursor", "/rulesets/a?list&limit=10&cursor=abc123", http.StatusOK, &rss, api.ListOptions{Limit: 10, Cursor: "abc123"}, nil}, {"NoResult", "/rulesets/?list", http.StatusOK, new(api.Rulesets), api.ListOptions{}, nil}, - {"InvalidCursor", "/rulesets/someprefix?list", http.StatusBadRequest, new(api.Rulesets), api.ListOptions{}, api.ErrInvalidCursor}, + {"InvalidCursor", "/rulesets/someprefix?list&cursor=abc123", http.StatusBadRequest, new(api.Rulesets), api.ListOptions{Cursor: "abc123"}, api.ErrInvalidCursor}, {"UnexpectedError", "/rulesets/someprefix?list", http.StatusInternalServerError, new(api.Rulesets), api.ListOptions{}, errors.New("unexpected error")}, {"InvalidLimit", "/rulesets/someprefix?list&limit=badlimit", http.StatusBadRequest, nil, api.ListOptions{}, nil}, } @@ -135,7 +135,7 @@ func TestAPI(t *testing.T) { assert.Equal(t, limit, strconv.Itoa(opt.Limit)) assert.Equal(t, cursor, opt.Cursor) assert.Equal(t, test.opt, opt) - return test.rss, err + return test.rss, test.err } defer func() { s.ListFn = nil }() @@ -323,6 +323,12 @@ func TestAPI(t *testing.T) { s.PutFn = func(context.Context, string, []*rule.Rule) (string, error) { return test.version, test.err } + s.GetFn = func(ctx context.Context, path string, version string) (*regula.Ruleset, error) { + return ®ula.Ruleset{ + Path: path, + Version: version, + }, nil + } defer func() { s.PutFn = nil }() var buf bytes.Buffer @@ -339,7 +345,7 @@ func TestAPI(t *testing.T) { var rs regula.Ruleset err := json.NewDecoder(w.Body).Decode(&rs) require.NoError(t, err) - require.EqualValues(t, test.version, rs) + require.Equal(t, test.version, rs.Version) } }) From 77c62d6a4607602ea0725f22b2e630c6d8643034 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 14:12:19 +0200 Subject: [PATCH 41/48] Split server tests --- http/server/api_test.go | 644 +++++++++++++++++++++------------------- 1 file changed, 332 insertions(+), 312 deletions(-) diff --git a/http/server/api_test.go b/http/server/api_test.go index 873d9df..edc13fe 100644 --- a/http/server/api_test.go +++ b/http/server/api_test.go @@ -21,370 +21,390 @@ import ( "github.com/stretchr/testify/require" ) -func TestAPI(t *testing.T) { - s := new(mock.RulesetService) - h := NewHandler(s, Config{ - WatchTimeout: 1 * time.Second, - }) - - r1 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - r2 := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} - - t.Run("Root", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/", nil) - h.ServeHTTP(w, r) - require.Equal(t, http.StatusNotFound, w.Code) - }) - - t.Run("Get", func(t *testing.T) { - rs1 := regula.Ruleset{ - Path: "a", - Version: "version1", - Rules: r1, - Versions: []string{"version1"}, - Signature: ®ula.Signature{ReturnType: "bool"}, - } - - rs2 := regula.Ruleset{ - Path: "a", - Version: "version2", - Rules: r1, - Versions: []string{"version1", "version2"}, - Signature: ®ula.Signature{ReturnType: "bool"}, - } - - tests := []struct { - name string - path string - status int - ruleset *regula.Ruleset - err error - }{ - {"Root", "/rulesets/a", http.StatusOK, &rs1, nil}, - {"NotFound", "/rulesets/b", http.StatusNotFound, &rs1, api.ErrRulesetNotFound}, - {"UnexpectedError", "/rulesets/a", http.StatusInternalServerError, &rs1, errors.New("unexpected error")}, - {"Specific version", "/rulesets/a?version=version2", http.StatusOK, &rs2, nil}, - } +// Rules +var r1 = []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} +var r2 = []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} + +// Rulesets +var rs1 = regula.Ruleset{ + Path: "a", + Version: "version1", + Rules: r1, + Versions: []string{"version1"}, + Signature: ®ula.Signature{ReturnType: "bool"}, +} - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - uu, err := url.Parse(test.path) - require.NoError(t, err) - version := uu.Query().Get("version") - s.GetFn = func(ctx context.Context, path, v string) (*regula.Ruleset, error) { - require.Equal(t, v, version) - return test.ruleset, test.err - } - defer func() { s.GetFn = nil }() - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", test.path, nil) - h.ServeHTTP(w, r) - - require.Equal(t, test.status, w.Code) - - if test.status == http.StatusOK { - var res regula.Ruleset - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - require.Len(t, res.Versions, len(test.ruleset.Versions)) - require.Equal(t, test.ruleset.Path, res.Path) - require.Equal(t, test.ruleset.Signature, res.Signature) - require.Equal(t, test.ruleset.Version, res.Version) - require.Equal(t, test.ruleset.Rules, res.Rules) - } - }) - } - }) +var rs2 = regula.Ruleset{ + Path: "a", + Version: "version2", + Rules: r1, + Versions: []string{"version1", "version2"}, + Signature: ®ula.Signature{ReturnType: "bool"}, +} - t.Run("List", func(t *testing.T) { - rss := api.Rulesets{ - Paths: []string{"aa", "bb"}, - Revision: "somerev", - Cursor: "somecursor", - } +func TestServer(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{}) - tests := []struct { - name string - path string - status int - rss *api.Rulesets - opt api.ListOptions - err error - }{ - {"OK", "/rulesets/?list", http.StatusOK, &rss, api.ListOptions{}, nil}, - {"WithLimitAndCursor", "/rulesets/a?list&limit=10&cursor=abc123", http.StatusOK, &rss, api.ListOptions{Limit: 10, Cursor: "abc123"}, nil}, - {"NoResult", "/rulesets/?list", http.StatusOK, new(api.Rulesets), api.ListOptions{}, nil}, - {"InvalidCursor", "/rulesets/someprefix?list&cursor=abc123", http.StatusBadRequest, new(api.Rulesets), api.ListOptions{Cursor: "abc123"}, api.ErrInvalidCursor}, - {"UnexpectedError", "/rulesets/someprefix?list", http.StatusInternalServerError, new(api.Rulesets), api.ListOptions{}, errors.New("unexpected error")}, - {"InvalidLimit", "/rulesets/someprefix?list&limit=badlimit", http.StatusBadRequest, nil, api.ListOptions{}, nil}, - } + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/", nil) + h.ServeHTTP(w, r) + require.Equal(t, http.StatusNotFound, w.Code) +} - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - uu, err := url.Parse(test.path) - require.NoError(t, err) - limit := uu.Query().Get("limit") - if limit == "" { - limit = "0" - } - cursor := uu.Query().Get("cursor") +func TestServerGet(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{}) + + tests := []struct { + name string + path string + status int + ruleset *regula.Ruleset + err error + }{ + {"Root", "/rulesets/a", http.StatusOK, &rs1, nil}, + {"NotFound", "/rulesets/b", http.StatusNotFound, &rs1, api.ErrRulesetNotFound}, + {"UnexpectedError", "/rulesets/a", http.StatusInternalServerError, &rs1, errors.New("unexpected error")}, + {"Specific version", "/rulesets/a?version=version2", http.StatusOK, &rs2, nil}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + uu, err := url.Parse(test.path) + require.NoError(t, err) + version := uu.Query().Get("version") + s.GetFn = func(ctx context.Context, path, v string) (*regula.Ruleset, error) { + require.Equal(t, v, version) + return test.ruleset, test.err + } + defer func() { s.GetFn = nil }() - s.ListFn = func(ctx context.Context, opt api.ListOptions) (*api.Rulesets, error) { - assert.Equal(t, limit, strconv.Itoa(opt.Limit)) - assert.Equal(t, cursor, opt.Cursor) - assert.Equal(t, test.opt, opt) - return test.rss, test.err - } - defer func() { s.ListFn = nil }() - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", test.path, nil) - h.ServeHTTP(w, r) - - require.Equal(t, test.status, w.Code) - - if test.status == http.StatusOK { - var res api.Rulesets - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - require.Equal(t, len(test.rss.Paths), len(res.Paths)) - for i := range test.rss.Paths { - require.EqualValues(t, test.rss.Paths[i], res.Paths[i]) - } - if len(test.rss.Paths) > 0 { - require.Equal(t, "somecursor", res.Cursor) - } - } - }) - } - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", test.path, nil) + h.ServeHTTP(w, r) - t.Run("Eval", func(t *testing.T) { - result := regula.EvalResult{Value: rule.StringValue("success"), Version: "123"} - - tests := []struct { - name string - path string - status int - mockFn func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) - }{ - {"OK", "/rulesets/path/to/my/ruleset?eval&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { - s, err := params.GetString("str") - require.NoError(t, err) - require.Equal(t, "str", s) - i, err := params.GetInt64("nb") - require.NoError(t, err) - require.Equal(t, int64(10), i) - b, err := params.GetBool("boolean") - require.NoError(t, err) - require.True(t, b) + require.Equal(t, test.status, w.Code) - return &result, nil - }}, - {"OK With version", "/rulesets/path/to/my/ruleset?eval&version=123&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { - s, err := params.GetString("str") + if test.status == http.StatusOK { + var res regula.Ruleset + err := json.NewDecoder(w.Body).Decode(&res) require.NoError(t, err) - require.Equal(t, "str", s) - return &result, nil - }}, - {"NOK - Ruleset not found", "/rulesets/path/to/my/ruleset?eval&foo=10", http.StatusNotFound, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { - return nil, rerrors.ErrRulesetNotFound - }}, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - resetStore(s) - - s.EvalFn = test.mockFn - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", test.path, nil) - h.ServeHTTP(w, r) + require.Len(t, res.Versions, len(test.ruleset.Versions)) + require.Equal(t, test.ruleset.Path, res.Path) + require.Equal(t, test.ruleset.Signature, res.Signature) + require.Equal(t, test.ruleset.Version, res.Version) + require.Equal(t, test.ruleset.Rules, res.Rules) + } + }) + } +} - require.Equal(t, test.status, w.Code) - }) - } +func TestServerList(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{}) + + rss := api.Rulesets{ + Paths: []string{"aa", "bb"}, + Revision: "somerev", + Cursor: "somecursor", + } + + tests := []struct { + name string + path string + status int + rss *api.Rulesets + opt api.ListOptions + err error + }{ + {"OK", "/rulesets/?list", http.StatusOK, &rss, api.ListOptions{}, nil}, + {"WithLimitAndCursor", "/rulesets/a?list&limit=10&cursor=abc123", http.StatusOK, &rss, api.ListOptions{Limit: 10, Cursor: "abc123"}, nil}, + {"NoResult", "/rulesets/?list", http.StatusOK, new(api.Rulesets), api.ListOptions{}, nil}, + {"InvalidCursor", "/rulesets/someprefix?list&cursor=abc123", http.StatusBadRequest, new(api.Rulesets), api.ListOptions{Cursor: "abc123"}, api.ErrInvalidCursor}, + {"UnexpectedError", "/rulesets/someprefix?list", http.StatusInternalServerError, new(api.Rulesets), api.ListOptions{}, errors.New("unexpected error")}, + {"InvalidLimit", "/rulesets/someprefix?list&limit=badlimit", http.StatusBadRequest, nil, api.ListOptions{}, nil}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + uu, err := url.Parse(test.path) + require.NoError(t, err) + limit := uu.Query().Get("limit") + if limit == "" { + limit = "0" + } + cursor := uu.Query().Get("cursor") - t.Run("NOK - errors", func(t *testing.T) { - errs := []error{ - rerrors.ErrParamNotFound, - rerrors.ErrParamTypeMismatch, - rerrors.ErrNoMatch, + s.ListFn = func(ctx context.Context, opt api.ListOptions) (*api.Rulesets, error) { + assert.Equal(t, limit, strconv.Itoa(opt.Limit)) + assert.Equal(t, cursor, opt.Cursor) + assert.Equal(t, test.opt, opt) + return test.rss, test.err } + defer func() { s.ListFn = nil }() - for _, e := range errs { - s.EvalFn = func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { - return nil, e - } + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", test.path, nil) + h.ServeHTTP(w, r) - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/rulesets/path/to/my/ruleset?eval&foo=10", nil) - h.ServeHTTP(w, r) + require.Equal(t, test.status, w.Code) - require.Equal(t, http.StatusBadRequest, w.Code) + if test.status == http.StatusOK { + var res api.Rulesets + err := json.NewDecoder(w.Body).Decode(&res) + require.NoError(t, err) + require.Equal(t, len(test.rss.Paths), len(res.Paths)) + for i := range test.rss.Paths { + require.EqualValues(t, test.rss.Paths[i], res.Paths[i]) + } + if len(test.rss.Paths) > 0 { + require.Equal(t, "somecursor", res.Cursor) + } } }) - }) + } +} - t.Run("Watch", func(t *testing.T) { - l := api.RulesetEvents{ - Events: []api.RulesetEvent{ - {Type: api.RulesetPutEvent, Path: "a", Rules: r1}, - {Type: api.RulesetPutEvent, Path: "b", Rules: r2}, - {Type: api.RulesetPutEvent, Path: "a", Rules: r2}, - }, - Revision: "rev", - } +func TestServerEval(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{}) + + result := regula.EvalResult{Value: rule.StringValue("success"), Version: "123"} + + tests := []struct { + name string + path string + status int + mockFn func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) + }{ + {"OK", "/rulesets/path/to/my/ruleset?eval&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { + s, err := params.GetString("str") + require.NoError(t, err) + require.Equal(t, "str", s) + i, err := params.GetInt64("nb") + require.NoError(t, err) + require.Equal(t, int64(10), i) + b, err := params.GetBool("boolean") + require.NoError(t, err) + require.True(t, b) - tests := []struct { - name string - path string - status int - es *api.RulesetEvents - err error - }{ - {"Root", "/rulesets/?watch", http.StatusOK, &l, nil}, - {"WithPrefix", "/rulesets/a?watch", http.StatusOK, &l, nil}, - {"NotFound", "/rulesets/a?watch", http.StatusNotFound, &l, api.ErrRulesetNotFound}, - {"Timeout", "/rulesets/?watch", http.StatusOK, nil, context.DeadlineExceeded}, - {"ContextCanceled", "/rulesets/?watch", http.StatusOK, nil, context.Canceled}, - } + return &result, nil + }}, + {"OK With version", "/rulesets/path/to/my/ruleset?eval&version=123&str=str&nb=10&boolean=true", http.StatusOK, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { + s, err := params.GetString("str") + require.NoError(t, err) + require.Equal(t, "str", s) + return &result, nil + }}, + {"NOK - Ruleset not found", "/rulesets/path/to/my/ruleset?eval&foo=10", http.StatusNotFound, func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { + return nil, rerrors.ErrRulesetNotFound + }}, + } - for _, test := range tests { - s.WatchFn = func(context.Context, string, string) (*api.RulesetEvents, error) { - return test.es, test.err - } - defer func() { s.WatchFn = nil }() + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resetStore(s) + + s.EvalFn = test.mockFn w := httptest.NewRecorder() r := httptest.NewRequest("GET", test.path, nil) h.ServeHTTP(w, r) require.Equal(t, test.status, w.Code) + }) + } - if test.status == http.StatusOK { - var res api.RulesetEvents - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - if test.es != nil { - require.Equal(t, len(test.es.Events), len(res.Events)) - for i := range l.Events { - require.Equal(t, l.Events[i], res.Events[i]) - } - } - } + t.Run("NOK - errors", func(t *testing.T) { + errs := []error{ + rerrors.ErrParamNotFound, + rerrors.ErrParamTypeMismatch, + rerrors.ErrNoMatch, } - t.Run("WithRevision", func(t *testing.T) { - s.WatchFn = func(ctx context.Context, prefix string, revision string) (*api.RulesetEvents, error) { - require.Equal(t, "a", prefix) - require.Equal(t, "somerev", revision) - return &l, nil + for _, e := range errs { + s.EvalFn = func(ctx context.Context, path, version string, params rule.Params) (*regula.EvalResult, error) { + return nil, e } - defer func() { s.WatchFn = nil }() w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/rulesets/a?watch&revision=somerev", nil) + r := httptest.NewRequest("GET", "/rulesets/path/to/my/ruleset?eval&foo=10", nil) h.ServeHTTP(w, r) - require.Equal(t, http.StatusOK, w.Code) + require.Equal(t, http.StatusBadRequest, w.Code) + } + }) +} - var res api.RulesetEvents - err := json.NewDecoder(w.Body).Decode(&res) - require.NoError(t, err) - require.Equal(t, len(l.Events), len(res.Events)) - for i := range l.Events { - require.Equal(t, l.Events[i], res.Events[i]) - } - }) +func TestServerWatch(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{ + WatchTimeout: 1 * time.Second, }) - t.Run("Put", func(t *testing.T) { - tests := []struct { - name string - path string - status int - version string - err error - }{ - {"OK", "/rulesets/a", http.StatusOK, "version", nil}, - {"NotModified", "/rulesets/a", http.StatusOK, "version", api.ErrRulesetNotModified}, - {"EmptyPath", "/rulesets/", http.StatusNotFound, "version", nil}, - {"StoreError", "/rulesets/a", http.StatusInternalServerError, "", errors.New("some error")}, - {"Bad ruleset name", "/rulesets/a", http.StatusBadRequest, "", new(api.ValidationError)}, - {"Bad param name", "/rulesets/a", http.StatusBadRequest, "", new(api.ValidationError)}, + l := api.RulesetEvents{ + Events: []api.RulesetEvent{ + {Type: api.RulesetPutEvent, Path: "a", Rules: r1}, + {Type: api.RulesetPutEvent, Path: "b", Rules: r2}, + {Type: api.RulesetPutEvent, Path: "a", Rules: r2}, + }, + Revision: "rev", + } + + tests := []struct { + name string + path string + status int + es *api.RulesetEvents + err error + }{ + {"Root", "/rulesets/?watch", http.StatusOK, &l, nil}, + {"WithPrefix", "/rulesets/a?watch", http.StatusOK, &l, nil}, + {"NotFound", "/rulesets/a?watch", http.StatusNotFound, &l, api.ErrRulesetNotFound}, + {"Timeout", "/rulesets/?watch", http.StatusOK, nil, context.DeadlineExceeded}, + {"ContextCanceled", "/rulesets/?watch", http.StatusOK, nil, context.Canceled}, + } + + for _, test := range tests { + s.WatchFn = func(context.Context, string, string) (*api.RulesetEvents, error) { + return test.es, test.err } + defer func() { s.WatchFn = nil }() - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - s.PutFn = func(context.Context, string, []*rule.Rule) (string, error) { - return test.version, test.err - } - s.GetFn = func(ctx context.Context, path string, version string) (*regula.Ruleset, error) { - return ®ula.Ruleset{ - Path: path, - Version: version, - }, nil - } - defer func() { s.PutFn = nil }() + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", test.path, nil) + h.ServeHTTP(w, r) - var buf bytes.Buffer - err := json.NewEncoder(&buf).Encode(r1) - require.NoError(t, err) + require.Equal(t, test.status, w.Code) - w := httptest.NewRecorder() - r := httptest.NewRequest("PUT", test.path, &buf) - h.ServeHTTP(w, r) + if test.status == http.StatusOK { + var res api.RulesetEvents + err := json.NewDecoder(w.Body).Decode(&res) + require.NoError(t, err) + if test.es != nil { + require.Equal(t, len(test.es.Events), len(res.Events)) + for i := range l.Events { + require.Equal(t, l.Events[i], res.Events[i]) + } + } + } + } - require.Equal(t, test.status, w.Code) + t.Run("WithRevision", func(t *testing.T) { + s.WatchFn = func(ctx context.Context, prefix string, revision string) (*api.RulesetEvents, error) { + require.Equal(t, "a", prefix) + require.Equal(t, "somerev", revision) + return &l, nil + } + defer func() { s.WatchFn = nil }() - if test.status == http.StatusOK { - var rs regula.Ruleset - err := json.NewDecoder(w.Body).Decode(&rs) - require.NoError(t, err) - require.Equal(t, test.version, rs.Version) - } - }) + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/rulesets/a?watch&revision=somerev", nil) + h.ServeHTTP(w, r) + require.Equal(t, http.StatusOK, w.Code) + + var res api.RulesetEvents + err := json.NewDecoder(w.Body).Decode(&res) + require.NoError(t, err) + require.Equal(t, len(l.Events), len(res.Events)) + for i := range l.Events { + require.Equal(t, l.Events[i], res.Events[i]) } }) +} - t.Run("Create", func(t *testing.T) { - sig := regula.Signature{ReturnType: "int64"} - - tests := []struct { - name string - path string - status int - err error - }{ - {"OK", "/rulesets/a", http.StatusCreated, nil}, - {"StoreError", "/rulesets/a", http.StatusInternalServerError, errors.New("some error")}, - {"Validation error", "/rulesets/a", http.StatusBadRequest, new(api.ValidationError)}, - } +func TestServerPut(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{}) + + tests := []struct { + name string + path string + status int + version string + err error + }{ + {"OK", "/rulesets/a", http.StatusOK, "version", nil}, + {"NotModified", "/rulesets/a", http.StatusOK, "version", api.ErrRulesetNotModified}, + {"EmptyPath", "/rulesets/", http.StatusNotFound, "version", nil}, + {"StoreError", "/rulesets/a", http.StatusInternalServerError, "", errors.New("some error")}, + {"Bad ruleset name", "/rulesets/a", http.StatusBadRequest, "", new(api.ValidationError)}, + {"Bad param name", "/rulesets/a", http.StatusBadRequest, "", new(api.ValidationError)}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s.PutFn = func(context.Context, string, []*rule.Rule) (string, error) { + return test.version, test.err + } + s.GetFn = func(ctx context.Context, path string, version string) (*regula.Ruleset, error) { + return ®ula.Ruleset{ + Path: path, + Version: version, + }, nil + } + defer func() { s.PutFn = nil }() - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - resetStore(s) - s.CreateFn = func(context.Context, string, *regula.Signature) error { - return test.err - } + var buf bytes.Buffer + err := json.NewEncoder(&buf).Encode(r1) + require.NoError(t, err) + + w := httptest.NewRecorder() + r := httptest.NewRequest("PUT", test.path, &buf) + h.ServeHTTP(w, r) + + require.Equal(t, test.status, w.Code) - var buf bytes.Buffer - err := json.NewEncoder(&buf).Encode(sig) + if test.status == http.StatusOK { + var rs regula.Ruleset + err := json.NewDecoder(w.Body).Decode(&rs) require.NoError(t, err) + require.Equal(t, test.version, rs.Version) + } + }) - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", test.path, &buf) - h.ServeHTTP(w, r) + } +} - require.Equal(t, test.status, w.Code) - }) - } +func TestServerCreate(t *testing.T) { + s := new(mock.RulesetService) + h := NewHandler(s, Config{ + WatchTimeout: 1 * time.Second, }) + + sig := regula.Signature{ReturnType: "int64"} + + tests := []struct { + name string + path string + status int + err error + }{ + {"OK", "/rulesets/a", http.StatusCreated, nil}, + {"StoreError", "/rulesets/a", http.StatusInternalServerError, errors.New("some error")}, + {"Validation error", "/rulesets/a", http.StatusBadRequest, new(api.ValidationError)}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resetStore(s) + s.CreateFn = func(context.Context, string, *regula.Signature) error { + return test.err + } + + var buf bytes.Buffer + err := json.NewEncoder(&buf).Encode(sig) + require.NoError(t, err) + + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", test.path, &buf) + h.ServeHTTP(w, r) + + require.Equal(t, test.status, w.Code) + }) + } } func resetStore(s *mock.RulesetService) { From e0e2beb766c05db7cd4faa004422df23ee41615b Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Tue, 21 May 2019 17:30:52 +0200 Subject: [PATCH 42/48] Remove debug print --- api/etcd/list.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/etcd/list.go b/api/etcd/list.go index c3742eb..26a2342 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -3,7 +3,6 @@ package etcd import ( "context" "encoding/base64" - "fmt" "strconv" "github.com/coreos/etcd/clientv3" @@ -57,7 +56,6 @@ func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Ru // if there are still paths left, generate a new cursor if len(rulesets.Paths) == opt.GetLimit() && resp.More { lastPath := rulesets.Paths[len(rulesets.Paths)-1] - fmt.Println("lastPath", lastPath) // we want to start immediately after the last key rulesets.Cursor = base64.URLEncoding.EncodeToString([]byte(lastPath + "\x00")) } From 0d6b1794447907348e3eafb349586ecb84eed608 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 22 May 2019 11:37:10 +0200 Subject: [PATCH 43/48] Apply suggestions from code review Co-Authored-By: Yasss --- api/etcd/eval_test.go | 2 +- api/etcd/list.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/etcd/eval_test.go b/api/etcd/eval_test.go index 6b24c45..49144f4 100644 --- a/api/etcd/eval_test.go +++ b/api/etcd/eval_test.go @@ -28,7 +28,7 @@ func TestEval(t *testing.T) { ruleset := createRuleset(t, s, "a", r) version := ruleset.Version - t.Run("OK", func(t *testing.T) { + t.Run("SpecificVersion", func(t *testing.T) { res, err := s.Eval(context.Background(), "a", version, regula.Params{ "id": "123", }) diff --git a/api/etcd/list.go b/api/etcd/list.go index 26a2342..186cdeb 100644 --- a/api/etcd/list.go +++ b/api/etcd/list.go @@ -38,7 +38,7 @@ func (s *RulesetService) List(ctx context.Context, opt api.ListOptions) (*api.Ru // limit the number of results opts = append(opts, clientv3.WithLimit(int64(opt.GetLimit()))) - // fetch signatures + // fetch signatures, the paths will be computed from signature keys resp, err := s.Client.KV.Get(ctx, key, opts...) if err != nil { return nil, errors.Wrap(err, "failed to fetch signatures") From d2484b47495224b1066b5f7c5da317510a170058 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 22 May 2019 11:45:26 +0200 Subject: [PATCH 44/48] Simplify etcd response reading --- api/etcd/get.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/etcd/get.go b/api/etcd/get.go index e6226db..3e43678 100644 --- a/api/etcd/get.go +++ b/api/etcd/get.go @@ -50,8 +50,9 @@ func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula } // the versions must be parsed from the keys returned by the second operation - versions := make([]string, len(resp.Responses[1].GetResponseRange().Kvs)) - for i, kv := range resp.Responses[1].GetResponseRange().Kvs { + kvs := resp.Responses[1].GetResponseRange().Kvs + versions := make([]string, len(kvs)) + for i, kv := range kvs { _, versions[i] = s.pathVersionFromKey(string(kv.Key)) } From 2dfb3d4d747a4515a8162c87b5112a84a1d46444 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 22 May 2019 11:45:58 +0200 Subject: [PATCH 45/48] Remove unnecessary docstring --- api/etcd/list_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/etcd/list_test.go b/api/etcd/list_test.go index 5248587..415fb6f 100644 --- a/api/etcd/list_test.go +++ b/api/etcd/list_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" ) -// List returns all rulesets rulesets or not depending on the query string. func TestList(t *testing.T) { rsTrue := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(true))} rsFalse := []*rule.Rule{rule.New(rule.True(), rule.BoolValue(false))} From f6f6d02dceda5a0116cc6856a7942c5d7f03d3e4 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 22 May 2019 11:56:53 +0200 Subject: [PATCH 46/48] Improve etcd get test --- api/etcd/get_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/api/etcd/get_test.go b/api/etcd/get_test.go index 077e8eb..9aa917c 100644 --- a/api/etcd/get_test.go +++ b/api/etcd/get_test.go @@ -48,8 +48,15 @@ func TestGet(t *testing.T) { require.Equal(t, rules2, ruleset2.Rules) require.Equal(t, sig, ruleset2.Signature) + // it should return the first version + rs, err := s.Get(context.Background(), path, ruleset1.Version) + require.NoError(t, err) + require.Equal(t, ruleset1.Path, rs.Path) + require.Equal(t, ruleset1.Version, rs.Version) + require.Equal(t, ruleset1.Rules, rs.Rules) + // it should return the second version - rs, err := s.Get(context.Background(), path, ruleset2.Version) + rs, err = s.Get(context.Background(), path, ruleset2.Version) require.NoError(t, err) require.Equal(t, ruleset2, rs) }) From 7f652a16c948f93d9868a9abe0941d7ef2b4fcfc Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 22 May 2019 15:58:15 +0200 Subject: [PATCH 47/48] Improve GetLimit behaviour --- api/service.go | 15 ++++++++++----- api/service_test.go | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/api/service.go b/api/service.go index a6d1ec1..fa2047b 100644 --- a/api/service.go +++ b/api/service.go @@ -38,15 +38,20 @@ type RulesetService interface { // ListOptions is used to customize the List output. type ListOptions struct { Limit int // If the Limit is lower or equal to 0 or greater than 100, it will be set to 50 by default. - Cursor string // pagination cursor + Cursor string // Pagination cursor. If empty the list starts from the beginning. } -// GetLimit returns a limit that is withing the 0 - 100 boundries. -// If the limit is incorrect, it returns 50. +// GetLimit returns a limit that is between 1 and 100. +// If Limit is lower of equal to zero, it returns 50. +// If Limit is bigger than 100, it returns 100. func (l *ListOptions) GetLimit() int { - if l.Limit <= 0 || l.Limit > 100 { - return 50 // TODO: make this one configurable + if l.Limit <= 0 { + return 50 } + if l.Limit > 100 { + return 100 + } + return l.Limit } diff --git a/api/service_test.go b/api/service_test.go index 02dc6f0..0bc963d 100644 --- a/api/service_test.go +++ b/api/service_test.go @@ -14,7 +14,7 @@ func TestListOptionsGetLimit(t *testing.T) { }{ {0, 50}, {-10, 50}, - {110, 50}, + {110, 100}, {70, 70}, } From cd5448a04a9d7c8db36112660a7b6cfbd4d26cb3 Mon Sep 17 00:00:00 2001 From: Asdine El Hrychy Date: Wed, 22 May 2019 15:59:11 +0200 Subject: [PATCH 48/48] Improve etcd get docstring --- api/etcd/get.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/etcd/get.go b/api/etcd/get.go index 3e43678..7dc21a8 100644 --- a/api/etcd/get.go +++ b/api/etcd/get.go @@ -11,7 +11,8 @@ import ( "github.com/pkg/errors" ) -// Get returns the ruleset related to the given path. +// Get returns the ruleset related to the given path.By default, it returns the latest one. +// It returns the related ruleset version if it's specified. func (s *RulesetService) Get(ctx context.Context, path, version string) (*regula.Ruleset, error) { if path == "" { return nil, api.ErrRulesetNotFound