Some prep work for further changes to the executor (#3874)

These changes will simplify some upcoming PRs. They include:

1) Moving the rule type engine into its own sub package.
2) Removing the rule type PB struct from the evaluation parameters. The
   alerting code currently accesses the rule type information by
   multiple paths (some of it is passed to the constructor for the
   alert, and other bits from the evaluation parameters). By picking a
   single way to pass the rule type around, changes for caching are
   simplified.

cmd/dev/app/rule_type/rttst.go

@@ -31,12 +31,12 @@ import (
 
 	serverconfig "github.com/stacklok/minder/internal/config/server"
 	"github.com/stacklok/minder/internal/db"
-	"github.com/stacklok/minder/internal/engine"
 	"github.com/stacklok/minder/internal/engine/actions"
 	"github.com/stacklok/minder/internal/engine/entities"
 	"github.com/stacklok/minder/internal/engine/errors"
 	"github.com/stacklok/minder/internal/engine/eval/rego"
 	engif "github.com/stacklok/minder/internal/engine/interfaces"
+	"github.com/stacklok/minder/internal/engine/rtengine"
 	"github.com/stacklok/minder/internal/logger"
 	"github.com/stacklok/minder/internal/profiles"
 	"github.com/stacklok/minder/internal/providers/credentials"
@@ -159,7 +159,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
 	off := "off"
 	profile.Alert = &off
 
-	rules, err := engine.GetRulesFromProfileOfType(profile, ruletype)
+	rules, err := rtengine.GetRulesFromProfileOfType(profile, ruletype)
 	if err != nil {
 		return fmt.Errorf("error getting relevant fragment: %w", err)
 	}
@@ -172,7 +172,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
 
 	// TODO: use cobra context here
 	ctx := context.Background()
-	eng, err := engine.NewRuleTypeEngine(ctx, ruletype, prov)
+	eng, err := rtengine.NewRuleTypeEngine(ctx, ruletype, prov)
 	if err != nil {
 		return fmt.Errorf("cannot create rule type engine: %w", err)
 	}
@@ -198,7 +198,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
 
 func runEvaluationForRules(
 	cmd *cobra.Command,
-	eng *engine.RuleTypeEngine,
+	eng *rtengine.RuleTypeEngine,
 	inf *entities.EntityInfoWrapper,
 	remediateStatus db.NullRemediationStatusTypes,
 	remMetadata pqtype.NullRawMessage,

internal/engine/actions/alert/alert.go

@@ -56,7 +56,7 @@ func NewRuleAlert(
 				Msg("provider is not a GitHub provider. Silently skipping alerts.")
 			return noop.NewNoopAlert(ActionType)
 		}
-		return security_advisory.NewSecurityAdvisoryAlert(ActionType, ruletype.GetSeverity(), alertCfg.GetSecurityAdvisory(), client)
+		return security_advisory.NewSecurityAdvisoryAlert(ActionType, ruletype, alertCfg.GetSecurityAdvisory(), client)
 	}
 
 	return nil, fmt.Errorf("unknown alert type: %s", alertCfg.GetType())

internal/engine/actions/alert/security_advisory/security_advisory.go

@@ -98,7 +98,7 @@ If you have any questions or believe that this evaluation is incorrect, please d
 type Alert struct {
 	actionType           interfaces.ActionType
 	cli                  provifv1.GitHub
-	sev                  *pb.Severity
+	ruleType             *pb.RuleType
 	saCfg                *pb.RuleType_Definition_Alert_AlertTypeSA
 	summaryTmpl          *htmltemplate.Template
 	descriptionTmpl      *htmltemplate.Template
@@ -126,14 +126,15 @@ type templateParamsSA struct {
 	RuleRemediation string
 	Name            string
 }
+
 type alertMetadata struct {
 	ID string `json:"ghsa_id,omitempty"`
 }
 
 // NewSecurityAdvisoryAlert creates a new security-advisory alert action
 func NewSecurityAdvisoryAlert(
 	actionType interfaces.ActionType,
-	sev *pb.Severity,
+	ruleType *pb.RuleType,
 	saCfg *pb.RuleType_Definition_Alert_AlertTypeSA,
 	cli provifv1.GitHub,
 ) (*Alert, error) {
@@ -160,7 +161,7 @@ func NewSecurityAdvisoryAlert(
 	return &Alert{
 		actionType:           actionType,
 		cli:                  cli,
-		sev:                  sev,
+		ruleType:             ruleType,
 		saCfg:                saCfg,
 		summaryTmpl:          sumT,
 		descriptionTmpl:      descT,
@@ -349,16 +350,16 @@ func (alert *Alert) getParamsForSecurityAdvisory(
 	// Get the severity
 	result.Template.Severity = alert.getSeverityString()
 	// Get the guidance
-	result.Template.Guidance = params.GetRuleType().Guidance
+	result.Template.Guidance = alert.ruleType.Guidance
 	// Get the rule type name
-	result.Template.Rule = params.GetRuleType().Name
+	result.Template.Rule = alert.ruleType.Name
 	// Get the profile name
 	result.Template.Profile = params.GetProfile().Name
 	// Get the rule name
 	result.Template.Name = params.GetRule().Name
 
 	// Check if remediation is available for the rule type
-	if params.GetRuleType().Def.Remediate != nil {
+	if alert.ruleType.Def.Remediate != nil {
 		result.Template.RuleRemediation = "already available"
 	} else {
 		result.Template.RuleRemediation = "not available yet"
@@ -386,7 +387,7 @@ func (alert *Alert) getParamsForSecurityAdvisory(
 
 func (alert *Alert) getSeverityString() string {
 	if alert.saCfg.Severity == "" {
-		ruleSev := alert.sev.GetValue().Enum().AsString()
+		ruleSev := alert.ruleType.Severity.GetValue().Enum().AsString()
 		if ruleSev == "info" || ruleSev == "unknown" {
 			return "low"
 		}

internal/engine/executor.go

@@ -30,6 +30,7 @@ import (
 	evalerrors "github.com/stacklok/minder/internal/engine/errors"
 	"github.com/stacklok/minder/internal/engine/ingestcache"
 	engif "github.com/stacklok/minder/internal/engine/interfaces"
+	"github.com/stacklok/minder/internal/engine/rtengine"
 	"github.com/stacklok/minder/internal/history"
 	minderlogger "github.com/stacklok/minder/internal/logger"
 	"github.com/stacklok/minder/internal/profiles"
@@ -190,7 +191,7 @@ func (e *executor) getEvaluator(
 	rule *pb.Profile_Rule,
 	hierarchy []uuid.UUID,
 	ingestCache ingestcache.Cache,
-) (*engif.EvalStatusParams, *RuleTypeEngine, *actions.RuleActionsEngine, error) {
+) (*engif.EvalStatusParams, *rtengine.RuleTypeEngine, *actions.RuleActionsEngine, error) {
 	// Create eval status params
 	params, err := e.createEvalStatusParams(ctx, inf, profile, rule)
 	if err != nil {
@@ -220,10 +221,10 @@ func (e *executor) getEvaluator(
 		return nil, nil, nil, fmt.Errorf("error parsing rule type ID: %w", err)
 	}
 	params.RuleTypeID = ruleTypeID
-	params.RuleType = ruleType
+	params.RuleTypeName = ruleType.Name
 
 	// Create the rule type engine
-	rte, err := NewRuleTypeEngine(ctx, ruleType, provider)
+	rte, err := rtengine.NewRuleTypeEngine(ctx, ruleType, provider)
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("error creating rule type engine: %w", err)
 	}

internal/engine/interfaces/interface.go

@@ -131,7 +131,7 @@ type EvalStatusParams struct {
 	Result           *Result
 	Profile          *pb.Profile
 	Rule             *pb.Profile_Rule
-	RuleType         *pb.RuleType
+	RuleTypeName     string
 	ProfileID        uuid.UUID
 	RepoID           uuid.NullUUID
 	ArtifactID       uuid.NullUUID
@@ -208,14 +208,19 @@ func (e *EvalStatusParams) GetRule() *pb.Profile_Rule {
 	return e.Rule
 }
 
+// GetRuleTypeID returns the rule type ID
+func (e *EvalStatusParams) GetRuleTypeID() uuid.UUID {
+	return e.RuleTypeID
+}
+
 // GetEvalStatusFromDb returns the evaluation status from the database
 func (e *EvalStatusParams) GetEvalStatusFromDb() *db.ListRuleEvaluationsByProfileIdRow {
 	return e.EvalStatusFromDb
 }
 
-// GetRuleType returns the rule type
-func (e *EvalStatusParams) GetRuleType() *pb.RuleType {
-	return e.RuleType
+// GetRuleTypeName returns the rule type name
+func (e *EvalStatusParams) GetRuleTypeName() string {
+	return e.RuleTypeName
 }
 
 // GetProfile returns the profile
@@ -240,7 +245,7 @@ func (e *EvalStatusParams) DecorateLogger(l zerolog.Logger) zerolog.Logger {
 		Str("profile_id", e.ProfileID.String()).
 		Str("rule_type", e.GetRule().GetType()).
 		Str("rule_name", e.GetRule().GetName()).
-		Str("rule_type_id", e.GetRuleType().GetId()).
+		Str("rule_type_id", e.GetRuleTypeID().String()).
 		Logger()
 	if e.RepoID.Valid {
 		outl = outl.With().Str("repository_id", e.RepoID.UUID.String()).Logger()
@@ -275,6 +280,7 @@ type ActionsParams interface {
 	GetActionsErr() evalerrors.ActionsError
 	GetEvalErr() error
 	GetEvalStatusFromDb() *db.ListRuleEvaluationsByProfileIdRow
-	GetRuleType() *pb.RuleType
+	GetRuleTypeName() string
 	GetProfile() *pb.Profile
+	GetRuleTypeID() uuid.UUID
 }

internal/engine/rule_type_engine.go → internal/engine/rtengine/engine.go

@@ -12,7 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package engine
+// Package rtengine contains the rule type engine
+package rtengine
 
 import (
 	"context"

internal/logger/telemetry_store.go

@@ -110,19 +110,14 @@ func (ts *TelemetryStore) AddRuleEval(
 		return
 	}
 
-	// Get rule type ID
-	ruleTypeID, err := uuid.Parse(evalInfo.GetRuleType().GetId())
-	if err != nil {
-		return
-	}
 	// Get profile ID
 	profileID, err := uuid.Parse(evalInfo.GetProfile().GetId())
 	if err != nil {
 		return
 	}
 
 	red := RuleEvalData{
-		RuleType:   RuleType{Name: evalInfo.GetRuleType().GetName(), ID: ruleTypeID},
+		RuleType:   RuleType{Name: evalInfo.GetRuleTypeName(), ID: evalInfo.GetRuleTypeID()},
 		Profile:    Profile{Name: evalInfo.GetProfile().GetName(), ID: profileID},
 		EvalResult: errors.EvalErrorAsString(evalInfo.GetEvalErr()),
 		Actions: map[interfaces.ActionType]ActionEvalData{

internal/logger/telemetry_store_test.go

@@ -53,10 +53,7 @@ func TestTelemetryStore_Record(t *testing.T) {
 				Name: "artifact_profile",
 				Id:   &testUUIDString,
 			}
-			ep.RuleType = &minderv1.RuleType{
-				Name: "artifact_signature",
-				Id:   &testUUIDString,
-			}
+			ep.RuleTypeName = "artifact_signature"
 			ep.SetEvalErr(enginerr.NewErrEvaluationFailed("evaluation failure reason"))
 			ep.SetActionsOnOff(map[engif.ActionType]engif.ActionOpt{
 				alert.ActionType:     engif.ActionOptOn,
@@ -83,10 +80,8 @@ func TestTelemetryStore_Record(t *testing.T) {
 				Name: "artifact_profile",
 				Id:   &testUUIDString,
 			}
-			ep.RuleType = &minderv1.RuleType{
-				Name: "artifact_signature",
-				Id:   &testUUIDString,
-			}
+			ep.RuleTypeName = "artifact_signature"
+			ep.RuleTypeID = testUUID
 			ep.SetEvalErr(enginerr.NewErrEvaluationFailed("evaluation failure reason"))
 			ep.SetActionsOnOff(map[engif.ActionType]engif.ActionOpt{
 				alert.ActionType:     engif.ActionOptOff,
@@ -139,7 +134,7 @@ func TestTelemetryStore_Record(t *testing.T) {
 		recordFunc: func(_ context.Context, _ engif.ActionsParams) {
 		},
 		expected:   `{"telemetry": "true"}`,
-		notPresent: []string{"project", "rules", "login_sha", "repository", "provider", "profile", "ruletype", "artifact", "pr"},
+		notPresent: []string{"project", "rules", "login_sha", "repository", "provider", "profile", "ruletypes", "artifact", "pr"},
 	}}
 
 	count := len(cases)

internal/profiles/rule_validator_test.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/stacklok/minder/internal/engine"
+	"github.com/stacklok/minder/internal/engine/rtengine"
 	"github.com/stacklok/minder/internal/profiles"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
 )
@@ -67,7 +67,7 @@ func TestExampleRulesAreValidatedCorrectly(t *testing.T) {
 			rval, err := profiles.NewRuleValidator(rt)
 			require.NoError(t, err, "failed to create rule validator for rule type %s", path)
 
-			rules, err := engine.GetRulesFromProfileOfType(pol, rt)
+			rules, err := rtengine.GetRulesFromProfileOfType(pol, rt)
 			require.NoError(t, err, "failed to get rules from profile for rule type %s", path)
 
 			t.Log("validating rules")