Skip to content

Commit

Permalink
fix: fix bug in evaluation of filters with filtersLogicalOperator=or
Browse files Browse the repository at this point in the history
Signed-off-by: jsvk <[email protected]>
  • Loading branch information
jsvk committed Dec 21, 2022
1 parent b9e4bfe commit 4cc7f1d
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 3 deletions.
11 changes: 8 additions & 3 deletions sensors/dependencies/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,16 @@ func filterEvent(filter *v1alpha1.EventDependencyFilter, operator v1alpha1.Logic
}

if operator == v1alpha1.OrLogicalOperator {
pass := (filter.Exprs != nil && exprFilter) ||
(filter.Data != nil && dataFilter) ||
(filter.Context != nil && ctxFilter) ||
(filter.Time != nil && timeFilter) ||
(filter.Script != "" && scriptFilter)

if len(errMessages) > 0 {
return exprFilter || dataFilter || ctxFilter || timeFilter || scriptFilter,
errors.New(strings.Join(errMessages, errMsgListSeparator))
return pass, errors.New(strings.Join(errMessages, errMsgListSeparator))
}
return exprFilter || dataFilter || ctxFilter || timeFilter || scriptFilter, nil
return pass, nil
}
return exprFilter && dataFilter && ctxFilter && timeFilter && scriptFilter, nil
}
Expand Down
69 changes: 69 additions & 0 deletions sensors/dependencies/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package dependencies

import (
"encoding/json"
"testing"
"time"

Expand Down Expand Up @@ -623,6 +624,74 @@ func TestFilter(t *testing.T) {
assert.True(t, pass)
})

t.Run("filtersLogicalOperator == 'or' with only a subset of filters specified", func(t *testing.T) {
filter := &v1alpha1.EventDependencyFilter{
Exprs: []v1alpha1.ExprFilter{
{
// C
Expr: `committer_email == "definitely-wrong"`, // this will be wrong
Fields: []v1alpha1.PayloadField{
{
Path: "body.head_commit.committer.email",
Name: "committer_email",
},
},
},
},
Data: []v1alpha1.DataFilter{ // these evaluate to false
{
Path: "body.ref",
Type: "string",
Value: []string{"definitely-wrong"},
},
{
Path: "body.repository.full_name",
Type: "string",
Value: []string{"foo/bar"},
},
{
Path: "[body.commits.#.modified.#()#,body.commits.#.added.#()#,body.commits.#.removed.#()#]|@flatten|@flatten",
Type: "string",
Value: []string{"^.*README.*$", "^.*service_metadata.*$"},
},
},
}

eventDataBytes, err := json.Marshal(map[string]interface{}{
"body": map[string]interface{}{
"ref": "foo",
"head_commit": map[string]interface{}{
"committer": map[string]interface{}{
"email": "[email protected]",
},
},
"repository": map[string]interface{}{
"full_name": "anything/anything",
},
},
})

assert.NoError(t, err)

// should return false because the two filters above evaluate to false
filtersLogicalOperator := v1alpha1.OrLogicalOperator

now := time.Now().UTC()
event := &v1alpha1.Event{
Context: &v1alpha1.EventContext{
Time: metav1.Time{
Time: time.Date(now.Year(), now.Month(), now.Day(), 16, 36, 34, 0, time.UTC),
},
},
Data: eventDataBytes,
}

pass, err := filterEvent(filter, filtersLogicalOperator, event)

assert.NoError(t, err)
assert.False(t, pass)
})

t.Run("test advanced logic: (A || B) || (C || D)", func(t *testing.T) {
// data filter: A || B == false
// expr filter: C || D == true
Expand Down

0 comments on commit 4cc7f1d

Please sign in to comment.