Skip to content

Commit

Permalink
Revert "Decouple Action Engine from Rule Type Engine (mindersec#3599)"
Browse files Browse the repository at this point in the history
This commit caused the actions to always be skipped.
  • Loading branch information
jhrozek committed Jun 17, 2024
1 parent 81fe44f commit 679571a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 37 deletions.
16 changes: 3 additions & 13 deletions cmd/dev/app/rule_type/rttst.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ 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"
Expand Down Expand Up @@ -171,15 +170,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
}

// TODO: use cobra context here
ctx := context.Background()
eng, err := engine.NewRuleTypeEngine(ctx, ruletype, prov)
if err != nil {
return fmt.Errorf("cannot create rule type engine: %w", err)
}
actionEngine, err := actions.NewRuleActions(ctx, profile, ruletype, prov)
if err != nil {
return fmt.Errorf("cannot create rule actions engine: %w", err)
}
eng, err := engine.NewRuleTypeEngine(context.Background(), profile, ruletype, prov)

inf := &entities.EntityInfoWrapper{
Entity: ent,
Expand All @@ -193,7 +184,7 @@ func testCmdRun(cmd *cobra.Command, _ []string) error {
return fmt.Errorf("no rules found with type %s", ruletype.Name)
}

return runEvaluationForRules(cmd, eng, inf, remediateStatus, remMetadata, rules, actionEngine)
return runEvaluationForRules(cmd, eng, inf, remediateStatus, remMetadata, rules)
}

func runEvaluationForRules(
Expand All @@ -203,7 +194,6 @@ func runEvaluationForRules(
remediateStatus db.NullRemediationStatusTypes,
remMetadata pqtype.NullRawMessage,
frags []*minderv1.Profile_Rule,
actionEngine *actions.RuleActionsEngine,
) error {
for idx := range frags {
frag := frags[idx]
Expand Down Expand Up @@ -237,7 +227,7 @@ func runEvaluationForRules(
evalStatus.SetEvalErr(eng.Eval(ctx, inf, evalStatus))

// Perform the actions, if any
evalStatus.SetActionsErr(ctx, actionEngine.DoActions(ctx, inf.Entity, evalStatus))
evalStatus.SetActionsErr(ctx, eng.Actions(ctx, inf, evalStatus))

if errors.IsActionFatalError(evalStatus.GetActionsErr().RemediateErr) {
cmd.Printf("Remediation failed with fatal error: %s", evalStatus.GetActionsErr().RemediateErr)
Expand Down
38 changes: 15 additions & 23 deletions internal/engine/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/rs/zerolog"

"github.com/stacklok/minder/internal/db"
"github.com/stacklok/minder/internal/engine/actions"
"github.com/stacklok/minder/internal/engine/actions/alert"
"github.com/stacklok/minder/internal/engine/actions/remediate"
"github.com/stacklok/minder/internal/engine/entities"
Expand Down Expand Up @@ -191,7 +190,7 @@ func (e *Executor) evalEntityEvent(ctx context.Context, inf *entities.EntityInfo
// Let's evaluate all the rules for this profile
err = profiles.TraverseRules(relevant, func(rule *pb.Profile_Rule) error {
// Get the engine evaluator for this rule type
evalParams, ruleEngine, actions, err := e.getEvaluator(
evalParams, rte, err := e.getEvaluator(
ctx, inf, provider, profile, rule, hierarchy, ingestCache)
if err != nil {
return err
Expand All @@ -201,14 +200,12 @@ func (e *Executor) evalEntityEvent(ctx context.Context, inf *entities.EntityInfo
defer e.updateLockLease(ctx, *inf.ExecutionID, evalParams)

// Evaluate the rule
evalErr := ruleEngine.Eval(ctx, inf, evalParams)
evalParams.SetEvalErr(rte.Eval(ctx, inf, evalParams))

// Perform actions, if any
actionsErr := actions.DoActions(ctx, inf.Entity, evalParams)
evalParams.SetActionsErr(ctx, rte.Actions(ctx, inf, evalParams))

// Log the evaluation
evalParams.SetEvalErr(evalErr)
evalParams.SetActionsErr(ctx, actionsErr)
logEval(ctx, inf, evalParams)

// Create or update the evaluation status
Expand Down Expand Up @@ -269,11 +266,11 @@ func (e *Executor) getEvaluator(
rule *pb.Profile_Rule,
hierarchy []uuid.UUID,
ingestCache ingestcache.Cache,
) (*engif.EvalStatusParams, *RuleTypeEngine, *actions.RuleActionsEngine, error) {
) (*engif.EvalStatusParams, *RuleTypeEngine, error) {
// Create eval status params
params, err := e.createEvalStatusParams(ctx, inf, profile, rule)
if err != nil {
return nil, nil, nil, fmt.Errorf("error creating eval status params: %w", err)
return nil, nil, fmt.Errorf("error creating eval status params: %w", err)
}

// Load Rule Class from database
Expand All @@ -284,39 +281,34 @@ func (e *Executor) getEvaluator(
Name: rule.Type,
})
if err != nil {
return nil, nil, nil, fmt.Errorf("error getting rule type when traversing profile %s: %w", params.ProfileID, err)
return nil, nil, fmt.Errorf("error getting rule type when traversing profile %s: %w", params.ProfileID, err)
}

// Parse the rule type
ruleType, err := ruletypes.RuleTypePBFromDB(&dbrt)
rt, err := ruletypes.RuleTypePBFromDB(&dbrt)
if err != nil {
return nil, nil, nil, fmt.Errorf("error parsing rule type when traversing profile %s: %w", params.ProfileID, err)
return nil, nil, fmt.Errorf("error parsing rule type when traversing profile %s: %w", params.ProfileID, err)
}

// Save the rule type uuid
ruleTypeID, err := uuid.Parse(*ruleType.Id)
ruleTypeID, err := uuid.Parse(*rt.Id)
if err != nil {
return nil, nil, nil, fmt.Errorf("error parsing rule type ID: %w", err)
return nil, nil, fmt.Errorf("error parsing rule type ID: %w", err)
}
params.RuleTypeID = ruleTypeID
params.RuleType = ruleType
params.RuleType = rt

// Create the rule type engine
rte, err := NewRuleTypeEngine(ctx, ruleType, provider)
rte, err := NewRuleTypeEngine(ctx, profile, rt, provider)
if err != nil {
return nil, nil, nil, fmt.Errorf("error creating rule type engine: %w", err)
return nil, nil, fmt.Errorf("error creating rule type engine: %w", err)
}

rte = rte.WithIngesterCache(ingestCache)

actionEngine, err := actions.NewRuleActions(ctx, profile, ruleType, provider)
if err != nil {
return nil, nil, nil, fmt.Errorf("cannot create rule actions engine: %w", err)
}

// All okay
params.SetActionsOnOff(actionEngine.GetOnOffState())
return params, rte, actionEngine, nil
params.SetActionsOnOff(rte.GetActionsOnOff())
return params, rte, nil
}

func (e *Executor) updateLockLease(
Expand Down
31 changes: 30 additions & 1 deletion internal/engine/rule_type_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/rs/zerolog"

"github.com/stacklok/minder/internal/engine/actions"
"github.com/stacklok/minder/internal/engine/entities"
enginerr "github.com/stacklok/minder/internal/engine/errors"
"github.com/stacklok/minder/internal/engine/eval"
Expand Down Expand Up @@ -65,16 +66,22 @@ type RuleTypeEngine struct {
// ruleEvaluator is the rule evaluator
ruleEvaluator engif.Evaluator

// actionsEngine is the rule actions engine
actionsEngine *actions.RuleActionsEngine

ruleValidator *profiles.RuleValidator

ruletype *minderv1.RuleType

//provider provinfv1.Provider

ingestCache ingestcache.Cache
}

// NewRuleTypeEngine creates a new rule type engine
func NewRuleTypeEngine(
ctx context.Context,
profile *minderv1.Profile,
ruletype *minderv1.RuleType,
provider provinfv1.Provider,
) (*RuleTypeEngine, error) {
Expand All @@ -93,15 +100,22 @@ func NewRuleTypeEngine(
return nil, fmt.Errorf("cannot create rule evaluator: %w", err)
}

ae, err := actions.NewRuleActions(ctx, profile, ruletype, provider)
if err != nil {
return nil, fmt.Errorf("cannot create rule actions engine: %w", err)
}

rte := &RuleTypeEngine{
Meta: RuleMeta{
Name: ruletype.Name,
},
ruleValidator: rval,
ingester: rdi,
ruleEvaluator: reval,
actionsEngine: ae,
ruletype: ruletype,
ingestCache: ingestcache.NewNoopCache(),
//cli: cli,
ingestCache: ingestcache.NewNoopCache(),
}

if ruletype.Context.Project != nil && *ruletype.Context.Project != "" {
Expand Down Expand Up @@ -177,6 +191,21 @@ func (r *RuleTypeEngine) Eval(
return err
}

// Actions runs all actions for the rule type engine against the given entity
func (r *RuleTypeEngine) Actions(
ctx context.Context,
inf *entities.EntityInfoWrapper,
params engif.ActionsParams,
) enginerr.ActionsError {
// Process actions
return r.actionsEngine.DoActions(ctx, inf.Entity, params)
}

// GetActionsOnOff returns the on/off state of the actions
func (r *RuleTypeEngine) GetActionsOnOff() map[engif.ActionType]engif.ActionOpt {
return r.actionsEngine.GetOnOffState()
}

// GetRulesFromProfileOfType returns the rules from the profile of the given type
func GetRulesFromProfileOfType(p *minderv1.Profile, rt *minderv1.RuleType) ([]*minderv1.Profile_Rule, error) {
contextualRules, err := profiles.GetRulesForEntity(p, minderv1.EntityFromString(rt.Def.InEntity))
Expand Down

0 comments on commit 679571a

Please sign in to comment.