Merge pull request #771 from stacklok/refactor-engie

small refactor: Move files around for testability

internal/engine/eval/errors/errors.go

@@ -0,0 +1,31 @@
+// Copyright 2023 Stacklok, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//	http://www.apache.role/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+// Package rule provides the CLI subcommand for managing rules
+
+// Package errors provides errors for the evaluator engine
+package errors
+
+import (
+	"errors"
+	"fmt"
+)
+
+// ErrEvaluationFailed is an error that occurs during evaluation of a rule.
+var ErrEvaluationFailed = errors.New("evaluation error")
+
+// NewErrEvaluationFailed creates a new evaluation error
+func NewErrEvaluationFailed(sfmt string, args ...any) error {
+	msg := fmt.Sprintf(sfmt, args...)
+	return fmt.Errorf("%w: %s", ErrEvaluationFailed, msg)
+}

internal/engine/eval/eval.go

@@ -19,9 +19,9 @@ package eval
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
+	"github.com/stacklok/mediator/internal/engine/eval/jq"
 	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
 )
 
@@ -30,15 +30,6 @@ type Evaluator interface {
 	Eval(ctx context.Context, policy map[string]any, obj any) error
 }
 
-// ErrEvaluationFailed is an error that occurs during evaluation of a rule.
-var ErrEvaluationFailed = errors.New("evaluation error")
-
-// NewErrEvaluationFailed creates a new evaluation error
-func NewErrEvaluationFailed(sfmt string, args ...any) error {
-	msg := fmt.Sprintf(sfmt, args...)
-	return fmt.Errorf("%w: %s", ErrEvaluationFailed, msg)
-}
-
 // NewRuleEvaluator creates a new rule data evaluator
 func NewRuleEvaluator(rt *pb.RuleType) (Evaluator, error) {
 	ing := rt.Def.GetEval()
@@ -53,7 +44,7 @@ func NewRuleEvaluator(rt *pb.RuleType) (Evaluator, error) {
 			return nil, fmt.Errorf("rule type engine missing rest configuration")
 		}
 
-		return NewJQEvaluator(ing.GetJq())
+		return jq.NewJQEvaluator(ing.GetJq())
 	default:
 		return nil, fmt.Errorf("unsupported rule type engine: %s", rt.Def.Eval.Type)
 	}

internal/engine/eval/jq.go → internal/engine/eval/jq/jq.go

@@ -13,25 +13,27 @@
 // limitations under the License.
 // Package rule provides the CLI subcommand for managing rules
 
-package eval
+// Package jq provides the jq policy evaluator
+package jq
 
 import (
 	"context"
 	"encoding/json"
 	"fmt"
 	"reflect"
 
+	evalerrors "github.com/stacklok/mediator/internal/engine/eval/errors"
 	"github.com/stacklok/mediator/internal/util"
 	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
 )
 
-// JQEvaluator is an Evaluator that uses the jq library to evaluate rules
-type JQEvaluator struct {
+// Evaluator is an Evaluator that uses the jq library to evaluate rules
+type Evaluator struct {
 	assertions []*pb.RuleType_Definition_Eval_JQComparison
 }
 
 // NewJQEvaluator creates a new JQ rule data evaluator
-func NewJQEvaluator(assertions []*pb.RuleType_Definition_Eval_JQComparison) (*JQEvaluator, error) {
+func NewJQEvaluator(assertions []*pb.RuleType_Definition_Eval_JQComparison) (*Evaluator, error) {
 	if len(assertions) == 0 {
 		return nil, fmt.Errorf("missing jq assertions")
 	}
@@ -55,13 +57,13 @@ func NewJQEvaluator(assertions []*pb.RuleType_Definition_Eval_JQComparison) (*JQ
 		}
 	}
 
-	return &JQEvaluator{
+	return &Evaluator{
 		assertions: assertions,
 	}, nil
 }
 
 // Eval calls the jq library to evaluate the rule
-func (jqe *JQEvaluator) Eval(ctx context.Context, pol map[string]any, obj any) error {
+func (jqe *Evaluator) Eval(ctx context.Context, pol map[string]any, obj any) error {
 	for idx := range jqe.assertions {
 		a := jqe.assertions[idx]
 		policyVal, err := util.JQGetValuesFromAccessor(ctx, a.Policy.Def, pol)
@@ -84,7 +86,7 @@ func (jqe *JQEvaluator) Eval(ctx context.Context, pol map[string]any, obj any) e
 				msg = fmt.Sprintf("%s\nassertion: %s", msg, string(marshalledAssertion))
 			}
 
-			return NewErrEvaluationFailed(msg)
+			return evalerrors.NewErrEvaluationFailed(msg)
 		}
 	}
 

internal/engine/eval/jq_test.go → internal/engine/eval/jq/jq_test.go

@@ -13,15 +13,16 @@
 // limitations under the License.
 // Package rule provides the CLI subcommand for managing rules
 
-package eval_test
+package jq_test
 
 import (
 	"context"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 
-	"github.com/stacklok/mediator/internal/engine/eval"
+	evalerrors "github.com/stacklok/mediator/internal/engine/eval/errors"
+	"github.com/stacklok/mediator/internal/engine/eval/jq"
 	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
 )
 
@@ -79,7 +80,7 @@ func TestNewJQEvaluatorValid(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got, err := eval.NewJQEvaluator(tt.args.assertions)
+			got, err := jq.NewJQEvaluator(tt.args.assertions)
 			assert.NoError(t, err, "Got unexpected error")
 			assert.NotNil(t, got, "Got unexpected nil")
 		})
@@ -187,7 +188,7 @@ func TestNewJQEvaluatorInvalid(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			got, err := eval.NewJQEvaluator(tt.args.assertions)
+			got, err := jq.NewJQEvaluator(tt.args.assertions)
 			assert.Error(t, err, "expected error")
 			assert.Nil(t, got, "expected nil")
 		})
@@ -296,7 +297,7 @@ func TestValidJQEvals(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			jqe, err := eval.NewJQEvaluator(tt.assertions)
+			jqe, err := jq.NewJQEvaluator(tt.assertions)
 			assert.NoError(t, err, "Got unexpected error")
 			assert.NotNil(t, jqe, "Got unexpected nil")
 
@@ -451,12 +452,12 @@ func TestValidJQEvalsFailed(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			jqe, err := eval.NewJQEvaluator(tt.assertions)
+			jqe, err := jq.NewJQEvaluator(tt.assertions)
 			assert.NoError(t, err, "Got unexpected error")
 			assert.NotNil(t, jqe, "Got unexpected nil")
 
 			err = jqe.Eval(context.Background(), tt.args.pol, tt.args.obj)
-			assert.ErrorIs(t, err, eval.ErrEvaluationFailed, "Got unexpected error")
+			assert.ErrorIs(t, err, evalerrors.ErrEvaluationFailed, "Got unexpected error")
 		})
 	}
 }
@@ -523,13 +524,13 @@ func TestInvalidJQEvals(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			jqe, err := eval.NewJQEvaluator(tt.assertions)
+			jqe, err := jq.NewJQEvaluator(tt.assertions)
 			assert.NoError(t, err, "Got unexpected error")
 			assert.NotNil(t, jqe, "Got unexpected nil")
 
 			err = jqe.Eval(context.Background(), tt.args.pol, tt.args.obj)
 			assert.Error(t, err, "Got unexpected error")
-			assert.NotErrorIs(t, err, eval.ErrEvaluationFailed, "Got unexpected error")
+			assert.NotErrorIs(t, err, evalerrors.ErrEvaluationFailed, "Got unexpected error")
 		})
 	}
 }

internal/engine/executor.go

@@ -27,7 +27,7 @@ import (
 	"github.com/go-playground/validator/v10"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
-	"github.com/stacklok/mediator/internal/engine/eval"
+	evalerrors "github.com/stacklok/mediator/internal/engine/eval/errors"
 	"github.com/stacklok/mediator/internal/events"
 	"github.com/stacklok/mediator/internal/util"
 	"github.com/stacklok/mediator/pkg/crypto"
@@ -608,7 +608,7 @@ func parseRepoID(repoID any) (int32, error) {
 }
 
 func errorAsEvalStatus(err error) db.EvalStatusTypes {
-	if errors.Is(err, eval.ErrEvaluationFailed) {
+	if errors.Is(err, evalerrors.ErrEvaluationFailed) {
 		return db.EvalStatusTypesFailure
 	} else if err != nil {
 		return db.EvalStatusTypesError

internal/engine/ingester/builtin.go → internal/engine/ingester/builtin/builtin.go

@@ -13,7 +13,8 @@
 // limitations under the License.
 // Package rule provides the CLI subcommand for managing rules
 
-package ingester
+// Package builtin provides the builtin ingestion engine
+package builtin
 
 import (
 	"context"
@@ -30,6 +31,11 @@ import (
 	"github.com/stacklok/mediator/pkg/rule_methods"
 )
 
+const (
+	// BuiltinRuleDataIngestType is the type of the builtin rule data ingest engine
+	BuiltinRuleDataIngestType = "builtin"
+)
+
 // BuiltinRuleDataIngest is the engine for a rule type that uses builtin methods
 type BuiltinRuleDataIngest struct {
 	builtinCfg  *pb.BuiltinType

internal/engine/ingester/ingester.go

@@ -23,6 +23,8 @@ import (
 
 	"google.golang.org/protobuf/reflect/protoreflect"
 
+	"github.com/stacklok/mediator/internal/engine/ingester/builtin"
+	"github.com/stacklok/mediator/internal/engine/ingester/rest"
 	pb "github.com/stacklok/mediator/pkg/generated/protobuf/go/mediator/v1"
 	ghclient "github.com/stacklok/mediator/pkg/providers/github"
 )
@@ -36,20 +38,19 @@ type Ingester interface {
 // type definition.
 func NewRuleDataIngest(rt *pb.RuleType, cli ghclient.RestAPI, access_token string) (Ingester, error) {
 	ing := rt.Def.GetIngest()
-	// TODO: make this more generic and/or use constants
 	switch rt.Def.Ingest.Type {
-	case "rest":
+	case rest.RestRuleDataIngestType:
 		if rt.Def.Ingest.GetRest() == nil {
 			return nil, fmt.Errorf("rule type engine missing rest configuration")
 		}
 
-		return NewRestRuleDataIngest(ing.GetRest(), cli)
+		return rest.NewRestRuleDataIngest(ing.GetRest(), cli)
 
-	case "builtin":
+	case builtin.BuiltinRuleDataIngestType:
 		if rt.Def.Ingest.GetBuiltin() == nil {
 			return nil, fmt.Errorf("rule type engine missing internal configuration")
 		}
-		return NewBuiltinRuleDataIngest(ing.GetBuiltin(), access_token)
+		return builtin.NewBuiltinRuleDataIngest(ing.GetBuiltin(), access_token)
 	default:
 		return nil, fmt.Errorf("unsupported rule type engine: %s", rt.Def.Ingest.Type)
 	}