From b668c939e0ebf5b99a4c0ab80863c96e625d3030 Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Thu, 25 Jul 2024 22:38:32 +0200 Subject: [PATCH] Integrate the selectors into the executor When an entity is not selected for a profile, let's save that into a profile-global status override and use that for all the statuses instead calling eval. Fixes: #3724 --- internal/engine/executor.go | 52 ++++++++++++++++++++++++++++++-- internal/engine/executor_test.go | 14 +++++++++ internal/service/service.go | 3 ++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/internal/engine/executor.go b/internal/engine/executor.go index 0cddcc9820..49e340497e 100644 --- a/internal/engine/executor.go +++ b/internal/engine/executor.go @@ -32,11 +32,13 @@ import ( "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/engine/selectors" "github.com/stacklok/minder/internal/history" minderlogger "github.com/stacklok/minder/internal/logger" "github.com/stacklok/minder/internal/profiles" "github.com/stacklok/minder/internal/profiles/models" "github.com/stacklok/minder/internal/providers/manager" + provsel "github.com/stacklok/minder/internal/providers/selectors" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" provinfv1 "github.com/stacklok/minder/pkg/providers/v1" ) @@ -55,6 +57,7 @@ type executor struct { historyService history.EvaluationHistoryService featureFlags openfeature.IClient profileStore profiles.ProfileStore + selBuilder selectors.SelectionBuilder } // NewExecutor creates a new executor @@ -65,6 +68,7 @@ func NewExecutor( historyService history.EvaluationHistoryService, featureFlags openfeature.IClient, profileStore profiles.ProfileStore, + selBuilder selectors.SelectionBuilder, ) Executor { return &executor{ querier: querier, @@ -73,6 +77,7 @@ func NewExecutor( historyService: historyService, featureFlags: featureFlags, profileStore: profileStore, + selBuilder: selBuilder, } } @@ -131,10 +136,14 @@ func (e *executor) EvalEntityEvent(ctx context.Context, inf *entities.EntityInfo return fmt.Errorf("error while retrieving profiles and rule instances: %w", err) } - // For each profile, evaluate each rule and store the outcome in the database + // For each profile, get the profile-override status. Then, if there is no profile-override status, + // evaluate each rule and store the outcome in the database or store the override status for all rules for _, profile := range profileAggregates { + + profileEvalStatus := e.profileEvalStatus(ctx, provider, inf, profile) + for _, rule := range profile.Rules { - if err := e.evaluateRule(ctx, inf, provider, &profile, &rule, ruleEngineCache); err != nil { + if err := e.evaluateRule(ctx, inf, provider, &profile, &rule, ruleEngineCache, profileEvalStatus); err != nil { return fmt.Errorf("error evaluating entity event: %w", err) } } @@ -150,6 +159,7 @@ func (e *executor) evaluateRule( profile *models.ProfileAggregate, rule *models.RuleInstance, ruleEngineCache rtengine.Cache, + profileEvalStatus error, ) error { // Create eval status params evalParams, err := e.createEvalStatusParams(ctx, inf, profile, rule) @@ -176,7 +186,12 @@ func (e *executor) evaluateRule( defer e.updateLockLease(ctx, *inf.ExecutionID, evalParams) // Evaluate the rule - evalErr := ruleEngine.Eval(ctx, inf, evalParams) + var evalErr error + if profileEvalStatus != nil { + evalErr = profileEvalStatus + } else { + evalErr = ruleEngine.Eval(ctx, inf, evalParams) + } evalParams.SetEvalErr(evalErr) // Perform actionEngine, if any @@ -190,6 +205,37 @@ func (e *executor) evaluateRule( return e.createOrUpdateEvalStatus(ctx, evalParams) } +func (e *executor) profileEvalStatus( + ctx context.Context, + provider provinfv1.Provider, + eiw *entities.EntityInfoWrapper, + aggregate models.ProfileAggregate, +) error { + // so far this function only handles selectors. In the future we can extend it to handle other + // profile-global evaluations + + selection, err := e.selBuilder.NewSelectionFromProfile(eiw.Type, aggregate.Selectors) + if err != nil { + return fmt.Errorf("error creating selection from profile: %w", err) + } + + selEnt := provsel.EntityToSelectorEntity(ctx, provider, eiw.Type, eiw.Entity) + if selEnt == nil { + return fmt.Errorf("error converting entity to selector entity") + } + + selected, err := selection.Select(selEnt) + if err != nil { + return fmt.Errorf("error selecting entity: %w", err) + } + + if !selected { + return evalerrors.NewErrEvaluationSkipped("entity not applicable due to profile selector") + } + + return nil +} + func (e *executor) updateLockLease( ctx context.Context, executionID uuid.UUID, diff --git a/internal/engine/executor_test.go b/internal/engine/executor_test.go index 52b886a4b5..b4fa11fa26 100644 --- a/internal/engine/executor_test.go +++ b/internal/engine/executor_test.go @@ -40,6 +40,7 @@ import ( "github.com/stacklok/minder/internal/engine/actions/alert" "github.com/stacklok/minder/internal/engine/actions/remediate" "github.com/stacklok/minder/internal/engine/entities" + mock_selectors "github.com/stacklok/minder/internal/engine/selectors/mock" "github.com/stacklok/minder/internal/flags" mockhistory "github.com/stacklok/minder/internal/history/mock" "github.com/stacklok/minder/internal/logger" @@ -357,6 +358,18 @@ default allow = true`, return fn(mockStore) }) + mockSelection := mock_selectors.NewMockSelection(ctrl) + mockSelection.EXPECT(). + Select(gomock.Any(), gomock.Any()). + Return(true, nil). + AnyTimes() + + mockSelectionBuilder := mock_selectors.NewMockSelectionBuilder(ctrl) + mockSelectionBuilder.EXPECT(). + NewSelectionFromProfile(gomock.Any(), gomock.Any()). + Return(mockSelection, nil). + AnyTimes() + executor := engine.NewExecutor( mockStore, providerManager, @@ -364,6 +377,7 @@ default allow = true`, historyService, &flags.FakeClient{}, profiles.NewProfileStore(mockStore), + mockSelectionBuilder, ) eiw := entities.NewEntityInfoWrapper(). diff --git a/internal/service/service.go b/internal/service/service.go index bd3796e18c..8e1e94ffab 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -36,6 +36,7 @@ import ( "github.com/stacklok/minder/internal/email/awsses" "github.com/stacklok/minder/internal/email/noop" "github.com/stacklok/minder/internal/engine" + "github.com/stacklok/minder/internal/engine/selectors" "github.com/stacklok/minder/internal/events" "github.com/stacklok/minder/internal/flags" "github.com/stacklok/minder/internal/history" @@ -190,6 +191,7 @@ func AllInOneServerService( } profileStore := profiles.NewProfileStore(store) + selEnv := selectors.NewEnv() // Register the executor to handle entity evaluations exec := engine.NewExecutor( @@ -199,6 +201,7 @@ func AllInOneServerService( history.NewEvaluationHistoryService(), featureFlagClient, profileStore, + selEnv, ) handler := engine.NewExecutorEventHandler(