builtin: Return evalerrors.ErrEvaluationSkipSilently in case the buil…

…tin evaluator doesn't match the entity

Returning `nil, nil` is wrong as it would be treated the same as
"evaluation was performed an no issues were found"

internal/engine/ingester/builtin/builtin.go

@@ -20,12 +20,12 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"log"
 	"reflect"
 	"strings"
 
 	"google.golang.org/protobuf/reflect/protoreflect"
 
+	evalerrors "github.com/stacklok/mediator/internal/engine/errors"
 	"github.com/stacklok/mediator/internal/util"
 	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
 	"github.com/stacklok/mediator/pkg/rule_methods"
@@ -39,6 +39,7 @@ const (
 // BuiltinRuleDataIngest is the engine for a rule type that uses builtin methods
 type BuiltinRuleDataIngest struct {
 	builtinCfg  *pb.BuiltinType
+	ruleMethods any
 	method      string
 	accessToken string
 }
@@ -52,14 +53,14 @@ func NewBuiltinRuleDataIngest(
 		builtinCfg:  builtinCfg,
 		accessToken: access_token,
 		method:      builtinCfg.GetMethod(),
+		ruleMethods: rule_methods.RuleMethods{},
 	}, nil
 }
 
 // Ingest calls the builtin method and populates the data to be returned
 func (idi *BuiltinRuleDataIngest) Ingest(ctx context.Context, ent protoreflect.ProtoMessage, params map[string]any) (any, error) {
 	// call internal method stored in pkg and method
-	rm := rule_methods.RuleMethods{}
-	value := reflect.ValueOf(rm)
+	value := reflect.ValueOf(idi.ruleMethods)
 	method := value.MethodByName(idi.method)
 
 	// Check if the method exists
@@ -70,12 +71,8 @@ func (idi *BuiltinRuleDataIngest) Ingest(ctx context.Context, ent protoreflect.P
 	matches, err := entityMatchesParams(ctx, ent, params)
 	if err != nil {
 		return nil, fmt.Errorf("cannot check if entity matches params: %w", err)
-	}
-
-	// TODO: this should be a warning
-	if !matches {
-		log.Printf("entity not matching parameters, skipping")
-		return nil, nil
+	} else if !matches {
+		return nil, evalerrors.ErrEvaluationSkipSilently
 	}
 
 	// call method

internal/engine/ingester/builtin/builtin_test.go

@@ -0,0 +1,32 @@
+package builtin
+
+import (
+	"context"
+	evalerrors "github.com/stacklok/mediator/internal/engine/errors"
+	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
+	"github.com/stretchr/testify/assert"
+	"testing"
+
+	"google.golang.org/protobuf/reflect/protoreflect"
+)
+
+type fakeMethods struct{}
+
+func (fm *fakeMethods) Test(ctx context.Context, ent protoreflect.ProtoMessage, params map[string]any) (any, error) {
+	return nil, nil
+}
+
+func TestNewBuiltinRuleDataIngestNoMatch(t *testing.T) {
+	bi, err := NewBuiltinRuleDataIngest(nil, "")
+	assert.NoError(t, err)
+
+	// set the rule methods to a fake
+	bi.ruleMethods = &fakeMethods{}
+	bi.method = "Test"
+
+	res, err := bi.Ingest(context.Background(), &pb.Artifact{}, map[string]any{
+		"foo": "bar",
+	})
+	assert.Equal(t, evalerrors.ErrEvaluationSkipSilently, err)
+	assert.Nil(t, res)
+}