From aeb998b3cb04f7569bceb9af5e44da423aa81f22 Mon Sep 17 00:00:00 2001 From: Simon Plourde Date: Tue, 25 Jun 2019 15:23:46 -0400 Subject: [PATCH] Fix entity_attributes matching (#3089) Signed-off-by: Simon Plourde --- CHANGELOG.md | 2 + backend/schedulerd/proxy_check_test.go | 78 +++++++++++++++----------- js/js.go | 19 +++++-- js/js_bench_test.go | 41 +++++++++++++- js/js_test.go | 44 --------------- 5 files changed, 98 insertions(+), 86 deletions(-) delete mode 100644 js/js_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f6cc600bdd..4628e116c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ POST & PUT requests instead of `204 No Content`. ### Fixed - Fixed a bug where events were not deleted when their corresponding entity was. +- Fixed the entity_attributes in proxy_requests so all attributes must match +instead of only one of them. ## [5.10.0] - 2019-06-18 diff --git a/backend/schedulerd/proxy_check_test.go b/backend/schedulerd/proxy_check_test.go index e18235c2df..5334072fc9 100644 --- a/backend/schedulerd/proxy_check_test.go +++ b/backend/schedulerd/proxy_check_test.go @@ -11,6 +11,32 @@ import ( ) func TestMatchEntities(t *testing.T) { + entity1 := &corev2.Entity{ + ObjectMeta: corev2.ObjectMeta{ + Name: "entity1", + Namespace: "default", + Labels: map[string]string{"proxy_type": "switch"}, + }, + EntityClass: "proxy", + System: corev2.System{Hostname: "foo.local"}, + } + entity2 := &corev2.Entity{ + ObjectMeta: corev2.ObjectMeta{ + Name: "entity2", + Namespace: "default", + Labels: map[string]string{"proxy_type": "sensor"}, + }, + Deregister: true, + EntityClass: "proxy", + } + entity3 := &corev2.Entity{ + ObjectMeta: corev2.ObjectMeta{ + Name: "entity3", + Namespace: "default", + }, + EntityClass: "agent", + } + tests := []struct { name string entityAttributes []string @@ -20,56 +46,40 @@ func TestMatchEntities(t *testing.T) { { name: "standard string attribute", entityAttributes: []string{`entity.name == "entity1"`}, - entities: []corev2.Resource{ - corev2.FixtureEntity("entity1"), - corev2.FixtureEntity("entity2"), - }, - want: []*corev2.Entity{ - corev2.FixtureEntity("entity1"), - }, + entities: []corev2.Resource{entity1, entity2, entity3}, + want: []*corev2.Entity{entity1}, }, { name: "standard bool attribute", - entityAttributes: []string{`entity.deregister == false`}, - entities: []corev2.Resource{ - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}, Deregister: false}, - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "bar", Namespace: "default"}, Deregister: true}, - }, - want: []*corev2.Entity{ - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}, Deregister: false}, - }, + entityAttributes: []string{`entity.deregister == true`}, + entities: []corev2.Resource{entity1, entity2, entity3}, + want: []*corev2.Entity{entity2}, }, { name: "nested standard attribute", entityAttributes: []string{`entity.system.hostname == "foo.local"`}, - entities: []corev2.Resource{ - &corev2.Entity{System: corev2.System{Hostname: "localhost"}}, - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}}, - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}, System: corev2.System{Hostname: "foo.local"}}, - }, - want: []*corev2.Entity{ - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}, System: corev2.System{Hostname: "foo.local"}}, - }, + entities: []corev2.Resource{entity1, entity2, entity3}, + want: []*corev2.Entity{entity1}, }, { name: "multiple matches", entityAttributes: []string{`entity.entity_class == "proxy"`}, - entities: []corev2.Resource{ - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}, EntityClass: "proxy"}, - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "bar", Namespace: "default"}, EntityClass: "agent"}, - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "baz", Namespace: "default"}, EntityClass: "proxy"}, - }, - want: []*corev2.Entity{ - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}, EntityClass: "proxy"}, - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "baz", Namespace: "default"}, EntityClass: "proxy"}, - }, + entities: []corev2.Resource{entity1, entity2, entity3}, + want: []*corev2.Entity{entity1, entity2}, }, { name: "invalid expression", entityAttributes: []string{`foo &&`}, - entities: []corev2.Resource{ - &corev2.Entity{ObjectMeta: corev2.ObjectMeta{Name: "foo", Namespace: "default"}}, + entities: []corev2.Resource{entity1, entity2, entity3}, + }, + { + name: "multiple entity attributes", + entityAttributes: []string{ + `entity.entity_class == "proxy"`, + `entity.labels.proxy_type == "sensor"`, }, + entities: []corev2.Resource{entity1, entity2, entity3}, + want: []*corev2.Entity{entity2}, }, } for _, tc := range tests { diff --git a/js/js.go b/js/js.go index e98dba6636..c0e5df7846 100644 --- a/js/js.go +++ b/js/js.go @@ -182,19 +182,28 @@ func MatchEntities(expressions []string, entities []interface{}) ([]bool, error) result, err := jsvm.Run(script) if err != nil { logger.WithError(err).Debugf("error executing entity filter (%s)", script.String()) - continue + filtered = false + break } - b, err := result.ToBoolean() + matches, err := result.ToBoolean() if err != nil { logger.WithError(err).Errorf("entity filter did not return bool (%s)", script.String()) - continue + filtered = false + break } - if b { - filtered = true + if !matches { + filtered = false break } + // Mark the entity as filtered, but continue with the next script + // (expression) until it went through all filters + filtered = true } + + // At this point, the entity will be marked as filtered only if matched all + // the expressions results = append(results, filtered) } + return results, nil } diff --git a/js/js_bench_test.go b/js/js_bench_test.go index fb935a6f7b..0468d3750b 100644 --- a/js/js_bench_test.go +++ b/js/js_bench_test.go @@ -3,13 +3,13 @@ package js_test import ( "testing" + corev2 "github.com/sensu/sensu-go/api/core/v2" "github.com/sensu/sensu-go/js" - "github.com/sensu/sensu-go/types" "github.com/sensu/sensu-go/types/dynamic" ) func BenchmarkCheckEval(b *testing.B) { - check := types.FixtureCheck("foo") + check := corev2.FixtureCheck("foo") for i := 0; i < b.N; i++ { synth := dynamic.Synthesize(check) params := map[string]interface{}{ @@ -20,7 +20,7 @@ func BenchmarkCheckEval(b *testing.B) { } func BenchmarkMetricsEval(b *testing.B) { - metrics := types.FixtureMetrics() + metrics := corev2.FixtureMetrics() for i := 0; i < b.N; i++ { synth := dynamic.Synthesize(metrics) params := map[string]interface{}{ @@ -29,3 +29,38 @@ func BenchmarkMetricsEval(b *testing.B) { _, _ = js.Evaluate("metrics.points.length > 0", params, nil) } } + +func BenchmarkMatchEntities1000(b *testing.B) { + entity := dynamic.Synthesize(corev2.FixtureEntity("foo")) + // non-matching expression to avoid short-circuiting behaviour + expression := "entity.system.arch == 'amd65'" + + entities := make([]interface{}, 100) + expressions := make([]string, 10) + + for i := range entities { + entities[i] = entity + } + for i := range expressions { + expressions[i] = expression + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = js.MatchEntities(expressions, entities) + } +} + +func BenchmarkEvaluate1000(b *testing.B) { + entity := map[string]interface{}{ + "entity": dynamic.Synthesize(corev2.FixtureEntity("foo")), + } + expression := "entity.system.arch == 'amd64'" + + b.ResetTimer() + for i := 0; i < b.N; i++ { + for j := 0; j < 1000; j++ { + _, _ = js.Evaluate(expression, entity, nil) + } + } +} diff --git a/js/js_test.go b/js/js_test.go deleted file mode 100644 index 964ddc6d97..0000000000 --- a/js/js_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package js_test - -import ( - "testing" - - corev2 "github.com/sensu/sensu-go/api/core/v2" - "github.com/sensu/sensu-go/js" - "github.com/sensu/sensu-go/types/dynamic" -) - -func BenchmarkMatchEntities1000(b *testing.B) { - entity := dynamic.Synthesize(corev2.FixtureEntity("foo")) - // non-matching expression to avoid short-circuiting behaviour - expression := "entity.system.arch == 'amd65'" - - entities := make([]interface{}, 100) - expressions := make([]string, 10) - - for i := range entities { - entities[i] = entity - } - for i := range expressions { - expressions[i] = expression - } - - b.ResetTimer() - for i := 0; i < b.N; i++ { - _, _ = js.MatchEntities(expressions, entities) - } -} - -func BenchmarkEvaluate1000(b *testing.B) { - entity := map[string]interface{}{ - "entity": dynamic.Synthesize(corev2.FixtureEntity("foo")), - } - expression := "entity.system.arch == 'amd64'" - - b.ResetTimer() - for i := 0; i < b.N; i++ { - for j := 0; j < 1000; j++ { - _, _ = js.Evaluate(expression, entity, nil) - } - } -}