From b9cd860bf29f0e3008964f27d05f2bd72449ad3d Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Wed, 22 Jan 2020 17:39:14 +0100 Subject: [PATCH] Improve script filtering --- api/compose/spec.json | 10 +++ api/compose/spec/automation.json | 10 +++ api/system/spec.json | 34 +++++--- api/system/spec/automation.json | 10 +++ compose/rest/automation.go | 2 + compose/rest/request/automation.go | 13 +++ docs/compose/README.md | 2 + docs/system/README.md | 2 + pkg/corredor/filter.go | 107 ++++++++++++++++++++++-- pkg/corredor/filter_test.go | 129 ++++++++++++++++++++++++++++- pkg/corredor/service.go | 9 +- pkg/corredor/util.go | 48 +---------- pkg/corredor/util_test.go | 56 ------------- system/rest/automation.go | 2 + system/rest/request/automation.go | 13 +++ 15 files changed, 317 insertions(+), 130 deletions(-) diff --git a/api/compose/spec.json b/api/compose/spec.json index e0a914107..b1b75a332 100644 --- a/api/compose/spec.json +++ b/api/compose/spec.json @@ -1625,6 +1625,11 @@ "path": "/", "parameters": { "get": [ + { + "name": "resourceTypePrefixes", + "type": "[]string", + "title": "Filter by resource prefix" + }, { "name": "resourceTypes", "type": "[]string", @@ -1635,6 +1640,11 @@ "type": "[]string", "title": "Filter by event type" }, + { + "name": "excludeInvalid", + "type": "bool", + "title": "Exclude scripts that can not be used (errors)" + }, { "name": "excludeClientScripts", "type": "bool", diff --git a/api/compose/spec/automation.json b/api/compose/spec/automation.json index 75daa16fb..e924f40cb 100644 --- a/api/compose/spec/automation.json +++ b/api/compose/spec/automation.json @@ -14,6 +14,11 @@ "Path": "/", "Parameters": { "get": [ + { + "name": "resourceTypePrefixes", + "title": "Filter by resource prefix", + "type": "[]string" + }, { "name": "resourceTypes", "title": "Filter by resource type", @@ -24,6 +29,11 @@ "title": "Filter by event type", "type": "[]string" }, + { + "name": "excludeInvalid", + "title": "Exclude scripts that can not be used (errors)", + "type": "bool" + }, { "name": "excludeClientScripts", "title": "Do not include client scripts", diff --git a/api/system/spec.json b/api/system/spec.json index 5434a66f4..3b6b78320 100644 --- a/api/system/spec.json +++ b/api/system/spec.json @@ -1695,24 +1695,34 @@ "parameters": { "get": [ { - "name": "resourceTypes", - "type": "[]string", - "title": "Filter by resource type" + "name": "resourceTypePrefixes", + "type": "[]string", + "title": "Filter by resource prefix" }, { - "name": "eventTypes", - "type": "[]string", - "title": "Filter by event type" + "name": "resourceTypes", + "type": "[]string", + "title": "Filter by resource type" }, { - "name": "excludeClientScripts", - "type": "bool", - "title": "Do not include client scripts" + "name": "eventTypes", + "type": "[]string", + "title": "Filter by event type" }, { - "name": "excludeServerScripts", - "type": "bool", - "title": "Do not include server scripts" + "name": "excludeInvalid", + "type": "bool", + "title": "Exclude scripts that can not be used (errors)" + }, + { + "name": "excludeClientScripts", + "type": "bool", + "title": "Do not include client scripts" + }, + { + "name": "excludeServerScripts", + "type": "bool", + "title": "Do not include server scripts" } ] } diff --git a/api/system/spec/automation.json b/api/system/spec/automation.json index a7bb4f586..951d842c5 100644 --- a/api/system/spec/automation.json +++ b/api/system/spec/automation.json @@ -14,6 +14,11 @@ "Path": "/", "Parameters": { "get": [ + { + "name": "resourceTypePrefixes", + "title": "Filter by resource prefix", + "type": "[]string" + }, { "name": "resourceTypes", "title": "Filter by resource type", @@ -24,6 +29,11 @@ "title": "Filter by event type", "type": "[]string" }, + { + "name": "excludeInvalid", + "title": "Exclude scripts that can not be used (errors)", + "type": "bool" + }, { "name": "excludeClientScripts", "title": "Do not include client scripts", diff --git a/compose/rest/automation.go b/compose/rest/automation.go index f11557624..02c7e84bd 100644 --- a/compose/rest/automation.go +++ b/compose/rest/automation.go @@ -26,6 +26,8 @@ func (ctrl *Automation) List(ctx context.Context, r *request.AutomationList) (in ctx, corredor.Service(), corredor.Filter{ + ResourceTypePrefixes: r.ResourceTypePrefixes, + ExcludeInvalid: r.ExcludeInvalid, ResourceTypes: r.ResourceTypes, EventTypes: r.EventTypes, ExcludeServerScripts: r.ExcludeServerScripts, diff --git a/compose/rest/request/automation.go b/compose/rest/request/automation.go index b74b67d22..4c3a0262a 100644 --- a/compose/rest/request/automation.go +++ b/compose/rest/request/automation.go @@ -32,8 +32,10 @@ var _ = multipart.FileHeader{} // Automation list request parameters type AutomationList struct { + ResourceTypePrefixes []string ResourceTypes []string EventTypes []string + ExcludeInvalid bool ExcludeClientScripts bool ExcludeServerScripts bool } @@ -45,8 +47,10 @@ func NewAutomationList() *AutomationList { func (r AutomationList) Auditable() map[string]interface{} { var out = map[string]interface{}{} + out["resourceTypePrefixes"] = r.ResourceTypePrefixes out["resourceTypes"] = r.ResourceTypes out["eventTypes"] = r.EventTypes + out["excludeInvalid"] = r.ExcludeInvalid out["excludeClientScripts"] = r.ExcludeClientScripts out["excludeServerScripts"] = r.ExcludeServerScripts @@ -80,6 +84,12 @@ func (r *AutomationList) Fill(req *http.Request) (err error) { post[name] = string(param[0]) } + if val, ok := urlQuery["resourceTypePrefixes[]"]; ok { + r.ResourceTypePrefixes = parseStrings(val) + } else if val, ok = urlQuery["resourceTypePrefixes"]; ok { + r.ResourceTypePrefixes = parseStrings(val) + } + if val, ok := urlQuery["resourceTypes[]"]; ok { r.ResourceTypes = parseStrings(val) } else if val, ok = urlQuery["resourceTypes"]; ok { @@ -92,6 +102,9 @@ func (r *AutomationList) Fill(req *http.Request) (err error) { r.EventTypes = parseStrings(val) } + if val, ok := get["excludeInvalid"]; ok { + r.ExcludeInvalid = parseBool(val) + } if val, ok := get["excludeClientScripts"]; ok { r.ExcludeClientScripts = parseBool(val) } diff --git a/docs/compose/README.md b/docs/compose/README.md index 210234ea2..1d1ca22bb 100644 --- a/docs/compose/README.md +++ b/docs/compose/README.md @@ -132,8 +132,10 @@ Warning: implode(): Invalid arguments passed in /private/tmp/Users/darh/Work.cru | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | +| resourceTypePrefixes | []string | GET | Filter by resource prefix | N/A | NO | | resourceTypes | []string | GET | Filter by resource type | N/A | NO | | eventTypes | []string | GET | Filter by event type | N/A | NO | +| excludeInvalid | bool | GET | Exclude scripts that can not be used (errors) | N/A | NO | | excludeClientScripts | bool | GET | Do not include client scripts | N/A | NO | | excludeServerScripts | bool | GET | Do not include server scripts | N/A | NO | diff --git a/docs/system/README.md b/docs/system/README.md index 264746c62..5fcc83bc8 100644 --- a/docs/system/README.md +++ b/docs/system/README.md @@ -336,8 +336,10 @@ Warning: implode(): Invalid arguments passed in /private/tmp/Users/darh/Work.cru | Parameter | Type | Method | Description | Default | Required? | | --------- | ---- | ------ | ----------- | ------- | --------- | +| resourceTypePrefixes | []string | GET | Filter by resource prefix | N/A | NO | | resourceTypes | []string | GET | Filter by resource type | N/A | NO | | eventTypes | []string | GET | Filter by event type | N/A | NO | +| excludeInvalid | bool | GET | Exclude scripts that can not be used (errors) | N/A | NO | | excludeClientScripts | bool | GET | Do not include client scripts | N/A | NO | | excludeServerScripts | bool | GET | Do not include server scripts | N/A | NO | diff --git a/pkg/corredor/filter.go b/pkg/corredor/filter.go index 7b6b8bfb3..5f7002e46 100644 --- a/pkg/corredor/filter.go +++ b/pkg/corredor/filter.go @@ -1,27 +1,118 @@ package corredor import ( + "github.com/cortezaproject/corteza-server/pkg/slice" "strings" ) type ( Filter struct { + ResourceTypePrefixes []string `json:"resourceTypePrefixes"` ResourceTypes []string `json:"resourceTypes"` EventTypes []string `json:"eventTypes"` ExcludeServerScripts bool `json:"excludeServerScripts"` ExcludeClientScripts bool `json:"excludeClientScripts"` + ExcludeInvalid bool `json:"excludeInvalid"` - Page uint `json:"page"` - PerPage uint `json:"perPage"` - Count uint `json:"count"` + //Page uint `json:"page"` + //PerPage uint `json:"perPage"` + Count uint `json:"count"` } ) -// PrefixResources adds service string (if not already there) to all resources -func (f *Filter) PrefixResources(service string) { - for i := range f.ResourceTypes { - if !strings.HasPrefix(f.ResourceTypes[i], service) { - f.ResourceTypes[i] = service + ":" + f.ResourceTypes[i] +func (f Filter) makeFilterFn() func(s *Script) (b bool, err error) { + collectTypes := func(tt ...*Trigger) (rr []string, ee []string) { + for _, t := range tt { + rr = append(rr, t.ResourceTypes...) + ee = append(ee, t.EventTypes...) } + + return + } + + return func(s *Script) (b bool, err error) { + if f.ExcludeInvalid { + if len(s.Errors) > 0 { + // Skip scripts with errors + return + } + } + + var ( + resourceTypes, eventTypes = collectTypes(s.Triggers...) + ) + + if !f.checkPrefixes(resourceTypes...) { + return + } + + if !f.checkEventTypes(eventTypes...) { + return + } + + if !f.checkResourceTypes(resourceTypes...) { + return + } + + return true, nil } } + +// parses resource type prefixes, adds ui: and service prefix if not present +func (f *Filter) procRTPrefixes(service string) { + var ( + hasService, hasUi bool + ) + + for i, rtp := range f.ResourceTypePrefixes { + if rtp == service { + hasService = true + } + + if strings.HasPrefix(rtp, "ui") { + hasUi = true + } + + // Check all requested resource-type prefixed and make sure + // all start with either "ui" or resourcePrefix + ":" + if !strings.HasPrefix(rtp, "ui:") && !strings.HasPrefix(rtp, service) { + f.ResourceTypePrefixes[i] = service + ":" + rtp + } + } + + if !hasService { + f.ResourceTypePrefixes = append(f.ResourceTypePrefixes, service) + } + + if !hasUi { + f.ResourceTypePrefixes = append(f.ResourceTypePrefixes, "ui:") + } +} + +func (f Filter) checkPrefixes(rr ...string) bool { + if len(f.ResourceTypePrefixes) == 0 { + return true + } + + if len(rr) == 0 { + return false + } + + for _, p := range f.ResourceTypePrefixes { + for _, r := range rr { + if strings.HasPrefix(r, p) { + return true + } + } + } + + return false +} + +func (f Filter) checkResourceTypes(rr ...string) bool { + return len(f.ResourceTypes) == 0 || len(slice.IntersectStrings(f.ResourceTypes, rr)) > 0 +} + +func (f Filter) checkEventTypes(ee ...string) bool { + return len(f.EventTypes) == 0 || len(slice.IntersectStrings(f.EventTypes, ee)) > 0 +} diff --git a/pkg/corredor/filter_test.go b/pkg/corredor/filter_test.go index a0cb1e723..3756326ac 100644 --- a/pkg/corredor/filter_test.go +++ b/pkg/corredor/filter_test.go @@ -6,12 +6,133 @@ import ( "github.com/stretchr/testify/assert" ) -func TestManualScriptFilterResourcePrefixing(t *testing.T) { +func TestFilterResourcePrefixing(t *testing.T) { f := &Filter{ - ResourceTypes: []string{"system", "system:one", "two"}, + ResourceTypePrefixes: []string{ + "system", + "system:one", + "two", + "ui:admin:foo", + "ui:admin", + }, } - f.PrefixResources("system") + f.procRTPrefixes("system") - assert.New(t).Equal(f.ResourceTypes, []string{"system", "system:one", "system:two"}) + assert.New(t).Equal( + f.ResourceTypePrefixes, + []string{ + "system", + "system:one", + "system:two", + "ui:admin:foo", + "ui:admin", + }, + ) + + f = &Filter{} + + f.procRTPrefixes("system") + + assert.New(t).Equal( + f.ResourceTypePrefixes, + []string{ + "system", + "ui:", + }, + ) +} + +func TestResourceTypesChecker(t *testing.T) { + var ( + a = assert.New(t) + ) + + // nothing requested in the filter + a.True(Filter{ResourceTypes: []string{}}.checkResourceTypes("c")) + + // no resources + a.False(Filter{ResourceTypes: []string{"a", "b"}}.checkResourceTypes()) + + // no resources, no filter + a.True(Filter{}.checkResourceTypes()) + + a.True(Filter{ResourceTypes: []string{"a"}}.checkResourceTypes("a", "b")) + + a.False(Filter{ResourceTypes: []string{"c"}}.checkResourceTypes("a", "b")) +} + +func TestEventTypesChecker(t *testing.T) { + var ( + a = assert.New(t) + ) + + // nothing requested in the filter + a.True(Filter{EventTypes: []string{}}.checkEventTypes("c")) + + // no resources + a.False(Filter{EventTypes: []string{"a", "b"}}.checkEventTypes()) + + // no resources, no filter + a.True(Filter{}.checkEventTypes()) + + a.True(Filter{EventTypes: []string{"a"}}.checkEventTypes("a", "b")) + + a.False(Filter{EventTypes: []string{"c"}}.checkEventTypes("a", "b")) +} + +func TestScriptFilterMaker(t *testing.T) { + var ( + s1 = &Script{ + Triggers: []*Trigger{ + &Trigger{ + EventTypes: []string{"ev"}, + ResourceTypes: []string{"res"}, + }, + }, + } + + s2 = &Script{ + Triggers: []*Trigger{ + &Trigger{ + EventTypes: []string{"foo"}, + ResourceTypes: []string{"bar"}, + }, + }, + } + + s3 = &Script{ + Triggers: []*Trigger{ + &Trigger{ + EventTypes: []string{"not-a-match"}, + ResourceTypes: []string{"not-a-match"}, + }, + }, + } + + a = assert.New(t) + strip = func(b bool, _ error) bool { return b } + + f func(s *Script) (b bool, err error) + ) + + f = Filter{}.makeFilterFn() + a.True(strip(f(s1))) + a.True(strip(f(s2))) + a.True(strip(f(s3))) + + f = Filter{ResourceTypes: []string{"res"}}.makeFilterFn() + a.True(strip(f(s1))) + a.False(strip(f(s2))) + a.False(strip(f(s3))) + + f = Filter{EventTypes: []string{"ev"}}.makeFilterFn() + a.True(strip(f(s1))) + a.False(strip(f(s2))) + a.False(strip(f(s3))) + + f = Filter{EventTypes: []string{"ev", "foo"}}.makeFilterFn() + a.True(strip(f(s1))) + a.True(strip(f(s2))) + a.False(strip(f(s3))) } diff --git a/pkg/corredor/service.go b/pkg/corredor/service.go index c220b9035..e0f1a9c6f 100644 --- a/pkg/corredor/service.go +++ b/pkg/corredor/service.go @@ -2,7 +2,6 @@ package corredor import ( "context" - "github.com/go-chi/chi/middleware" "github.com/pkg/errors" "go.uber.org/zap" @@ -188,6 +187,8 @@ func (svc service) Find(ctx context.Context, filter Filter) (out ScriptSet, f Fi out = append(out, tmp...) } + f.Count = uint(len(out)) + return } @@ -195,7 +196,7 @@ func (svc service) Find(ctx context.Context, filter Filter) (out ScriptSet, f Fi // that (after basic filtering) also does RBAC check for each script func (svc service) makeScriptFilter(ctx context.Context, f Filter) func(s *Script) (b bool, err error) { var ( - base = makeScriptFilter(f) + base = f.makeFilterFn() ) return func(s *Script) (b bool, err error) { @@ -203,9 +204,7 @@ func (svc service) makeScriptFilter(ctx context.Context, f Filter) func(s *Scrip return } - b = svc.canExec(ctx, s.Name) - - return + return svc.canExec(ctx, s.Name), nil } } diff --git a/pkg/corredor/util.go b/pkg/corredor/util.go index c31aaa7ae..a519aa130 100644 --- a/pkg/corredor/util.go +++ b/pkg/corredor/util.go @@ -4,10 +4,8 @@ import ( "context" "encoding/json" "fmt" - "github.com/pkg/errors" - "github.com/cortezaproject/corteza-server/pkg/eventbus" - "github.com/cortezaproject/corteza-server/pkg/slice" + "github.com/pkg/errors" ) type ( @@ -93,50 +91,10 @@ func encodeArguments(args map[string]string, key string, val interface{}) (err e return } -// Creates a filter fn for script filtering -func makeScriptFilter(f Filter) func(s *Script) (b bool, err error) { - return func(s *Script) (b bool, err error) { - b = true - if len(f.ResourceTypes) > 0 { - // Filtering by resource type, - // at least one of the script's triggers should match - b = false - for _, t := range s.Triggers { - if len(slice.IntersectStrings(f.ResourceTypes, t.ResourceTypes)) > 0 { - b = true - } - } - - if !b { - // No match by resource type, break - return - } - } - - if len(f.EventTypes) > 0 { - // Filtering by event type, - // at least one of the script's triggers should match - b = false - for _, t := range s.Triggers { - if len(slice.IntersectStrings(f.EventTypes, t.EventTypes)) > 0 { - b = true - } - } - - if !b { - // No match by event type, break - return - } - } - - // Not explicitly filtered - return - } -} - // GenericListHandler returns filtered list of scripts func GenericListHandler(ctx context.Context, svc *service, f Filter, resourcePrefix string) (p *automationListSetPayload, err error) { - f.PrefixResources(resourcePrefix) + f.procRTPrefixes(resourcePrefix) + p = &automationListSetPayload{} p.Set, p.Filter, err = svc.Find(ctx, f) return p, err diff --git a/pkg/corredor/util_test.go b/pkg/corredor/util_test.go index bf06f5e6e..121966b9f 100644 --- a/pkg/corredor/util_test.go +++ b/pkg/corredor/util_test.go @@ -100,59 +100,3 @@ func TestArgEncoding(t *testing.T) { "obj": `{"A":"A"}`, }, args) } - -func TestScriptFilterMaker(t *testing.T) { - var ( - s1 = &Script{ - Triggers: []*Trigger{ - &Trigger{ - EventTypes: []string{"ev"}, - ResourceTypes: []string{"res"}, - }, - }, - } - - s2 = &Script{ - Triggers: []*Trigger{ - &Trigger{ - EventTypes: []string{"foo"}, - ResourceTypes: []string{"bar"}, - }, - }, - } - - s3 = &Script{ - Triggers: []*Trigger{ - &Trigger{ - EventTypes: []string{"not-a-match"}, - ResourceTypes: []string{"not-a-match"}, - }, - }, - } - - a = assert.New(t) - strip = func(b bool, _ error) bool { return b } - - f func(s *Script) (b bool, err error) - ) - - f = makeScriptFilter(Filter{}) - a.True(strip(f(s1))) - a.True(strip(f(s2))) - a.True(strip(f(s3))) - - f = makeScriptFilter(Filter{ResourceTypes: []string{"res"}}) - a.True(strip(f(s1))) - a.False(strip(f(s2))) - a.False(strip(f(s3))) - - f = makeScriptFilter(Filter{EventTypes: []string{"ev"}}) - a.True(strip(f(s1))) - a.False(strip(f(s2))) - a.False(strip(f(s3))) - - f = makeScriptFilter(Filter{EventTypes: []string{"ev", "foo"}}) - a.True(strip(f(s1))) - a.True(strip(f(s2))) - a.False(strip(f(s3))) -} diff --git a/system/rest/automation.go b/system/rest/automation.go index d8e499118..961039a44 100644 --- a/system/rest/automation.go +++ b/system/rest/automation.go @@ -26,6 +26,8 @@ func (ctrl *Automation) List(ctx context.Context, r *request.AutomationList) (in ctx, corredor.Service(), corredor.Filter{ + ResourceTypePrefixes: r.ResourceTypePrefixes, + ExcludeInvalid: r.ExcludeInvalid, ResourceTypes: r.ResourceTypes, EventTypes: r.EventTypes, ExcludeServerScripts: r.ExcludeServerScripts, diff --git a/system/rest/request/automation.go b/system/rest/request/automation.go index b74b67d22..4c3a0262a 100644 --- a/system/rest/request/automation.go +++ b/system/rest/request/automation.go @@ -32,8 +32,10 @@ var _ = multipart.FileHeader{} // Automation list request parameters type AutomationList struct { + ResourceTypePrefixes []string ResourceTypes []string EventTypes []string + ExcludeInvalid bool ExcludeClientScripts bool ExcludeServerScripts bool } @@ -45,8 +47,10 @@ func NewAutomationList() *AutomationList { func (r AutomationList) Auditable() map[string]interface{} { var out = map[string]interface{}{} + out["resourceTypePrefixes"] = r.ResourceTypePrefixes out["resourceTypes"] = r.ResourceTypes out["eventTypes"] = r.EventTypes + out["excludeInvalid"] = r.ExcludeInvalid out["excludeClientScripts"] = r.ExcludeClientScripts out["excludeServerScripts"] = r.ExcludeServerScripts @@ -80,6 +84,12 @@ func (r *AutomationList) Fill(req *http.Request) (err error) { post[name] = string(param[0]) } + if val, ok := urlQuery["resourceTypePrefixes[]"]; ok { + r.ResourceTypePrefixes = parseStrings(val) + } else if val, ok = urlQuery["resourceTypePrefixes"]; ok { + r.ResourceTypePrefixes = parseStrings(val) + } + if val, ok := urlQuery["resourceTypes[]"]; ok { r.ResourceTypes = parseStrings(val) } else if val, ok = urlQuery["resourceTypes"]; ok { @@ -92,6 +102,9 @@ func (r *AutomationList) Fill(req *http.Request) (err error) { r.EventTypes = parseStrings(val) } + if val, ok := get["excludeInvalid"]; ok { + r.ExcludeInvalid = parseBool(val) + } if val, ok := get["excludeClientScripts"]; ok { r.ExcludeClientScripts = parseBool(val) }