Fix entity_attributes matching (sensu#3089)

Signed-off-by: Simon Plourde <[email protected]>

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
 

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 {

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
 }

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)
+		}
+	}
+}