diff --git a/internal/engine/actions/actions.go b/internal/engine/actions/actions.go index e1f6052728..5686d739af 100644 --- a/internal/engine/actions/actions.go +++ b/internal/engine/actions/actions.go @@ -1,8 +1,8 @@ // SPDX-FileCopyrightText: Copyright 2023 The Minder Authors // SPDX-License-Identifier: Apache-2.0 -// Package actions provide necessary interfaces and implementations for -// processing actions, such as remediation and alerts. +// Package actions provides the rule actions engine responsible for +// processing remediation and alert actions based on evaluation results. package actions import ( @@ -15,8 +15,6 @@ import ( "github.com/rs/zerolog" "google.golang.org/protobuf/reflect/protoreflect" - dbadapter "github.com/mindersec/minder/internal/adapters/db" - "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/engine/actions/alert" "github.com/mindersec/minder/internal/engine/actions/remediate" "github.com/mindersec/minder/internal/engine/actions/remediate/pull_request" @@ -28,25 +26,24 @@ import ( provinfv1 "github.com/mindersec/minder/pkg/providers/v1" ) -// RuleActionsEngine is the engine responsible for processing all actions i.e., remediation and alerts +// RuleActionsEngine is responsible for executing remediation and alert actions type RuleActionsEngine struct { actions map[engif.ActionType]engif.Action } -// NewRuleActions creates a new rule actions engine +// NewRuleActions creates a new RuleActionsEngine func NewRuleActions( ctx context.Context, ruletype *minderv1.RuleType, provider provinfv1.Provider, actionConfig *models.ActionConfiguration, ) (*RuleActionsEngine, error) { - // Create the remediation engine + remEngine, err := remediate.NewRuleRemediator(ruletype, provider, actionConfig.Remediate) if err != nil { return nil, fmt.Errorf("cannot create rule remediator: %w", err) } - // Create the alert engine alertEngine, err := alert.NewRuleAlert(ctx, ruletype, provider, actionConfig.Alert) if err != nil { return nil, fmt.Errorf("cannot create rule alerter: %w", err) @@ -60,21 +57,18 @@ func NewRuleActions( }, nil } -// DoActions processes all actions i.e., remediation and alerts +// DoActions executes remediation and alert actions func (rae *RuleActionsEngine) DoActions( ctx context.Context, ent protoreflect.ProtoMessage, params engif.ActionsParams, ) enginerr.ActionsError { - // Get logger logger := zerolog.Ctx(ctx) - // Default to skipping all actions - result := getDefaultResult(ctx) + result := getDefaultResult() skipRemediate := true skipAlert := true - // Verify the remediate action engine is available and get its status - on/off/dry-run remediateEngine, ok := rae.actions[remediate.ActionType] if !ok { logger.Error().Str("action_type", string(remediate.ActionType)).Msg("not found") @@ -83,7 +77,6 @@ func (rae *RuleActionsEngine) DoActions( skipRemediate = rae.isSkippable(ctx, remediate.ActionType, params.GetEvalErr()) } - // Verify the alert action engine is available and get its status - on/off/dry-run _, ok = rae.actions[alert.ActionType] if !ok { logger.Error().Str("action_type", string(alert.ActionType)).Msg("not found") @@ -92,27 +85,59 @@ func (rae *RuleActionsEngine) DoActions( skipAlert = rae.isSkippable(ctx, alert.ActionType, params.GetEvalErr()) } - // Try remediating + var prev *PreviousEval + if row := params.GetEvalStatusFromDb(); row != nil { + prev = &PreviousEval{ + RemediationStatus: RemediationStatus(row.RemStatus), + AlertStatus: AlertStatus(row.AlertStatus), + RemediationMeta: &row.RemMetadata, + AlertMeta: &row.AlertMetadata, + } + } + status := mapEvalStatus(params.GetEvalErr()) + + var remMeta *json.RawMessage + var alertMeta *json.RawMessage + + if prev != nil { + remMeta = getRemediationMeta(prev) + alertMeta = getAlertMeta(prev) + } + if !skipRemediate { - // Decide if we should remediate - cmd := shouldRemediate(params.GetEvalStatusFromDb(), params.GetEvalErr()) - // Run remediation - result.RemediateMeta, result.RemediateErr = rae.processAction(ctx, remediate.ActionType, cmd, ent, params, - getRemediationMeta(params.GetEvalStatusFromDb())) + cmd := shouldRemediate(prev, status) + result.RemediateMeta, result.RemediateErr = rae.processAction( + ctx, + remediate.ActionType, + cmd, + ent, + params, + remMeta, + ) } - // Try alerting if !skipAlert { - // Decide if we should alert - cmd := shouldAlert(params.GetEvalStatusFromDb(), params.GetEvalErr(), result.RemediateErr, remediateEngine.Type()) - // Run alerting - result.AlertMeta, result.AlertErr = rae.processAction(ctx, alert.ActionType, cmd, ent, params, - getAlertMeta(params.GetEvalStatusFromDb())) + cmd := shouldAlert( + prev, + status, + result.RemediateErr, + remediateEngine.Type(), + ) + + result.AlertMeta, result.AlertErr = rae.processAction( + ctx, + alert.ActionType, + cmd, + ent, + params, + alertMeta, + ) } + return result } -// processAction runs the action engine for the given action type, and also sanity checks the result of the action +// processAction safely executes an action func (rae *RuleActionsEngine) processAction( ctx context.Context, actionType engif.ActionType, @@ -120,88 +145,86 @@ func (rae *RuleActionsEngine) processAction( ent protoreflect.ProtoMessage, params engif.ActionsParams, metadata *json.RawMessage, -) (jmsg json.RawMessage, finalErr error) { +) (res json.RawMessage, err error) { defer func() { if r := recover(); r != nil { - zerolog.Ctx(ctx).Error().Interface("recovered", r). + zerolog.Ctx(ctx).Error(). + Interface("recovered", r). Bytes("stack", debug.Stack()). Msg("panic in action execution") - finalErr = enginerr.ErrInternal + + err = enginerr.ErrInternal } }() - zerolog.Ctx(ctx).Debug().Str("action", string(actionType)).Str("cmd", string(cmd)).Msg("invoking action") - // Get action engine + action := rae.actions[actionType] - // Return the result of the action return action.Do(ctx, cmd, ent, params, metadata) } -// shouldRemediate returns the action command for remediation taking into account previous evaluations -func shouldRemediate(prevEvalFromDb *db.ListRuleEvaluationsByProfileIdRow, evalErr error) engif.ActionCmd { - // Get current evaluation status - newEval := dbadapter.ErrorAsEvalStatus(evalErr) - - // Get previous Remediation status - prevRemediation := db.RemediationStatusTypesSkipped - if prevEvalFromDb != nil { - prevRemediation = prevEvalFromDb.RemStatus +// shouldRemediate determines the remediation action command. +// taking into account the current evaluation result and the +// previous remediation state. +func shouldRemediate(prevEval *PreviousEval, evalStatus EvalStatus) engif.ActionCmd { + // Default previous remediation status + prevRemediation := RemediationStatusSkipped + if prevEval != nil { + prevRemediation = prevEval.RemediationStatus } - // Start evaluation scenarios - - // Case 1 - Do not try to be smart about it by doing nothing if the evaluation status has not changed + switch evalStatus { + case EvalStatusError: + // TODO: Historically, error may have fallen through to success behavior. + // This change treats error as no-op. Confirm intended behavior. + return engif.ActionCmdDoNothing - // Proceed with use cases where the evaluation changed - switch newEval { - case db.EvalStatusTypesError: - case db.EvalStatusTypesSuccess: + case EvalStatusSuccess: // Case 2 - Evaluation changed from something else to ERROR -> Remediation should be OFF // Case 3 - Evaluation changed from something else to PASSING -> Remediation should be OFF - // The Remediation should be OFF (if it wasn't already) - if db.RemediationStatusTypesSkipped != prevRemediation { + // If remediation was previously active, turn it off. + if prevRemediation != RemediationStatusSkipped { return engif.ActionCmdOff } - // We should do nothing if remediation was already skipped + // DoNothing means we keep the current state unchanged, rather than explicitly turning it OFF. return engif.ActionCmdDoNothing - case db.EvalStatusTypesFailure: + + case EvalStatusFailure: // Case 4 - Evaluation has changed from something else to FAILED -> Remediation should be ON - // We should remediate only if the previous remediation was skipped, so we don't risk endless remediation loops - if db.RemediationStatusTypesSkipped == prevRemediation { + // We only trigger remediation if the previous remediation was skipped. + // This prevents repeated remediation attempts and avoids endless remediation loops + // where the system keeps trying to remediate an already-handled failure. + if prevRemediation == RemediationStatusSkipped { return engif.ActionCmdOn } - // Do nothing if the Remediation is something else other than skipped, i.e. pending, success, error, etc. return engif.ActionCmdDoNothing - case db.EvalStatusTypesSkipped: - case db.EvalStatusTypesPending: + + case EvalStatusSkipped, EvalStatusPending: return engif.ActionCmdDoNothing } - // Default to do nothing return engif.ActionCmdDoNothing } -// shouldAlert returns the action command for alerting taking into account previous evaluations +// shouldAlert returns the action command for alerting, +// based on the current evaluation result, previous alert state, +// and remediation outcome. func shouldAlert( - prevEvalFromDb *db.ListRuleEvaluationsByProfileIdRow, - evalErr error, + prevEval *PreviousEval, + evalStatus EvalStatus, remErr error, remType string, ) engif.ActionCmd { - // Get current evaluation status - newEval := dbadapter.ErrorAsEvalStatus(evalErr) - // Get previous Alert status - prevAlert := db.AlertStatusTypesSkipped - if prevEvalFromDb != nil { - prevAlert = prevEvalFromDb.AlertStatus + // Default previous alert status + prevAlert := AlertStatusSkipped + if prevEval != nil { + prevAlert = prevEval.AlertStatus } - // Start evaluation scenarios - // Case 1 - Successful remediation of a type that is not PR is considered instant. if remType != pull_request.RemediateType && remErr == nil { - // If this is the case either skip alerting or turn it off if it was on - if prevAlert != db.AlertStatusTypesOff { + // If remediation succeeded, we turn alert OFF. + // This effectively skips alerting (or turns it off) when remediation resolves the issue immediately. + if prevAlert != AlertStatusOff { return engif.ActionCmdOff } return engif.ActionCmdDoNothing @@ -210,99 +233,85 @@ func shouldAlert( // Case 2 - Do not try to be smart about it by doing nothing if the evaluation status has not changed // Proceed with use cases where the evaluation changed - switch newEval { - case db.EvalStatusTypesError: - case db.EvalStatusTypesFailure: - // Case 3 - Evaluation changed from something else to ERROR -> Alert should be ON - // Case 4 - Evaluation has changed from something else to FAILED -> Alert should be ON - // The Alert should be on (if it wasn't already) - if db.AlertStatusTypesOn != prevAlert { + + // The alert logic attempts to interpret transitions between previous and current states, + // rather than simply mirroring the current evaluation result. This helps avoid noisy or + // redundant alerts and ensures we react only to meaningful state changes. + + switch evalStatus { + + case EvalStatusError: + // TODO: Historically, error may have fallen through to success behavior. + // This change treats error as no-op. Confirm intended behavior. + return engif.ActionCmdDoNothing + + // Case 3 - Evaluation changed from something else to ERROR -> Alert should be ON + // Case 4 - Evaluation has changed from something else to FAILED -> Alert should be ON + case EvalStatusFailure: + if prevAlert != AlertStatusOn { return engif.ActionCmdOn } - // We should do nothing if alert was already turned on return engif.ActionCmdDoNothing - case db.EvalStatusTypesSuccess: + // Case 5 - Evaluation changed from something else to PASSING -> Alert should be OFF + case EvalStatusSuccess: // Case 5 - Evaluation changed from something else to PASSING -> Alert should be OFF - // The Alert should be turned OFF (if it wasn't already) - if db.AlertStatusTypesOff != prevAlert { + if prevAlert != AlertStatusOff { return engif.ActionCmdOff } - // We should do nothing if the Alert is already OFF return engif.ActionCmdDoNothing - case db.EvalStatusTypesSkipped: - case db.EvalStatusTypesPending: + + case EvalStatusSkipped, EvalStatusPending: return engif.ActionCmdDoNothing } - // Default to do nothing return engif.ActionCmdDoNothing } -// isSkippable returns true if the action should be skipped +// isSkippable checks if action should be skipped func (rae *RuleActionsEngine) isSkippable(ctx context.Context, actionType engif.ActionType, evalErr error) bool { - var skipAction bool - - logger := zerolog.Ctx(ctx).Info(). - Str("eval_status", string(dbadapter.ErrorAsEvalStatus(evalErr))). - Str("action", string(actionType)) - - // Get the profile option set for this action type action, ok := rae.actions[actionType] if !ok { - // If the action is not found, definitely skip it - logger.Msg("action type not found, skipping") + zerolog.Ctx(ctx).Info(). + Str("eval_status", string(mapEvalStatus(evalErr))). + Str("action", string(actionType)). + Msg("action type not found, skipping") return true } - // Check the action option + switch action.GetOnOffState() { case models.ActionOptOff: - // Action is off, skip - logger.Msg("action is off, skipping") return true case models.ActionOptUnknown: - // Action is unknown, skip - logger.Msg("unknown action option, skipping") return true case models.ActionOptDryRun, models.ActionOptOn: - // Action is on or dry-run, do not skip yet. Check the evaluation error - skipAction = - // rule evaluation was skipped, skip action too - errors.Is(evalErr, interfaces.ErrEvaluationSkipped) || - // rule evaluation was skipped silently, skip action - errors.Is(evalErr, enginerr.ErrEvaluationSkipSilently) + // Some actions in ON or DRY-RUN mode may still be skipped if the evaluation + // explicitly returns a "skip" result. This ensures we respect evaluation intent. + return errors.Is(evalErr, interfaces.ErrEvaluationSkipped) || + errors.Is(evalErr, enginerr.ErrEvaluationSkipSilently) } - logger.Bool("skip_action", skipAction).Msg("action skip decision") - // Everything else, do not skip - return skipAction + + return false } -// getRemediationMeta returns the json.RawMessage from the database type, empty if not valid -func getRemediationMeta(prevEvalFromDb *db.ListRuleEvaluationsByProfileIdRow) *json.RawMessage { - if prevEvalFromDb != nil { - return &prevEvalFromDb.RemMetadata +// getRemediationMeta returns remediation metadata from previous evaluation. +func getRemediationMeta(prevEval *PreviousEval) *json.RawMessage { + if prevEval != nil { + return prevEval.RemediationMeta } return nil } -// getAlertMeta returns the json.RawMessage from the database type, empty if not valid -func getAlertMeta(prevEvalFromDb *db.ListRuleEvaluationsByProfileIdRow) *json.RawMessage { - if prevEvalFromDb != nil { - return &prevEvalFromDb.AlertMetadata +// getAlertMeta returns alert metadata from previous evaluation. +func getAlertMeta(prevEval *PreviousEval) *json.RawMessage { + if prevEval != nil { + return prevEval.AlertMeta } return nil } -// getDefaultResult returns the default result for the action engine -func getDefaultResult(ctx context.Context) enginerr.ActionsError { - // Get logger - logger := zerolog.Ctx(ctx) +func getDefaultResult() enginerr.ActionsError { + m := json.RawMessage("{}") - // Even though meta is an empty json struct by default, there's no risk of overwriting - // any existing meta entry since we don't upsert in case of conflict while skipping - m, err := json.Marshal(&map[string]any{}) - if err != nil { - logger.Error().Err(err).Msg("error marshaling empty json.RawMessage") - } return enginerr.ActionsError{ RemediateErr: enginerr.ErrActionSkipped, AlertErr: enginerr.ErrActionSkipped, @@ -310,3 +319,24 @@ func getDefaultResult(ctx context.Context) enginerr.ActionsError { AlertMeta: m, } } + +// mapEvalStatus converts evaluation error into engine EvalStatus. +func mapEvalStatus(err error) EvalStatus { + if err == nil { + return EvalStatusSuccess + } + + // skipped cases + if errors.Is(err, enginerr.ErrEvaluationSkipSilently) || + errors.Is(err, interfaces.ErrEvaluationSkipped) { + return EvalStatusSkipped + } + + // failure case (CORRECT detection) + if errors.Is(err, interfaces.ErrEvaluationFailed) { + return EvalStatusFailure + } + + // everything else = error + return EvalStatusError +} diff --git a/internal/engine/actions/actions_test.go b/internal/engine/actions/actions_test.go index 17ed1d7283..da35f0614f 100644 --- a/internal/engine/actions/actions_test.go +++ b/internal/engine/actions/actions_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" - "github.com/mindersec/minder/internal/db" "github.com/mindersec/minder/internal/engine/actions/remediate/pull_request" engif "github.com/mindersec/minder/internal/engine/interfaces" enginerr "github.com/mindersec/minder/pkg/engine/errors" @@ -19,82 +18,91 @@ func TestShouldRemediate(t *testing.T) { t.Parallel() tests := []struct { - name string - prevEval db.RemediationStatusTypes - nilRow bool - evalErr error - expected engif.ActionCmd + name string + prevStatus RemediationStatus + hasPrev bool + evalErr error + expected engif.ActionCmd }{ // Happy path: eval success { - name: "eval success, prev success -> off", - prevEval: db.RemediationStatusTypesSuccess, - evalErr: nil, - expected: engif.ActionCmdOff, + name: "eval success, prev success -> off", + prevStatus: RemediationStatusSuccess, + hasPrev: true, + evalErr: nil, + expected: engif.ActionCmdOff, }, { - name: "eval success, prev skipped -> do nothing", - prevEval: db.RemediationStatusTypesSkipped, - evalErr: nil, - expected: engif.ActionCmdDoNothing, + name: "eval success, prev skipped -> do nothing", + prevStatus: RemediationStatusSkipped, + hasPrev: true, + evalErr: nil, + expected: engif.ActionCmdDoNothing, }, // Happy path: eval failure triggers remediation { - name: "eval failure, prev skipped -> on", - prevEval: db.RemediationStatusTypesSkipped, - evalErr: enginerr.NewErrEvaluationFailed("failed"), - expected: engif.ActionCmdOn, + name: "eval failure, prev skipped -> on", + prevStatus: RemediationStatusSkipped, + hasPrev: true, + evalErr: enginerr.NewErrEvaluationFailed("failed"), + expected: engif.ActionCmdOn, }, { - name: "eval failure, prev success -> do nothing", - prevEval: db.RemediationStatusTypesSuccess, - evalErr: enginerr.NewErrEvaluationFailed("failed"), - expected: engif.ActionCmdDoNothing, + name: "eval failure, prev success -> do nothing", + prevStatus: RemediationStatusSuccess, + hasPrev: true, + evalErr: enginerr.NewErrEvaluationFailed("failed"), + expected: engif.ActionCmdDoNothing, }, // Expected errors: eval error cases { - name: "eval error, prev skipped -> do nothing", - prevEval: db.RemediationStatusTypesSkipped, - evalErr: errors.New("random error"), - expected: engif.ActionCmdDoNothing, + name: "eval error, prev skipped -> do nothing", + prevStatus: RemediationStatusSkipped, + hasPrev: true, + evalErr: errors.New("random error"), + expected: engif.ActionCmdDoNothing, }, { // NOTE: EvalStatusTypesError has an empty case body in shouldRemediate, // so eval errors fall through to the default DoNothing. This may be a bug // (see the comment on cases Error/Success in shouldRemediate). - name: "eval error, prev success -> do nothing", - prevEval: db.RemediationStatusTypesSuccess, - evalErr: errors.New("random error"), - expected: engif.ActionCmdDoNothing, + name: "eval error, prev success -> do nothing", + prevStatus: RemediationStatusSuccess, + hasPrev: true, + evalErr: errors.New("random error"), + expected: engif.ActionCmdDoNothing, }, { - name: "eval failure, prev error -> do nothing", - prevEval: db.RemediationStatusTypesError, - evalErr: enginerr.NewErrEvaluationFailed("failed"), - expected: engif.ActionCmdDoNothing, + name: "eval failure, prev error -> do nothing", + prevStatus: RemediationStatusError, + hasPrev: true, + evalErr: enginerr.NewErrEvaluationFailed("failed"), + expected: engif.ActionCmdDoNothing, }, { - name: "eval error, prev error -> do nothing", - prevEval: db.RemediationStatusTypesError, - evalErr: errors.New("random error"), - expected: engif.ActionCmdDoNothing, + name: "eval error, prev error -> do nothing", + prevStatus: RemediationStatusError, + hasPrev: true, + evalErr: errors.New("random error"), + expected: engif.ActionCmdDoNothing, }, // Edge cases { - name: "eval skipped -> do nothing", - prevEval: db.RemediationStatusTypesSkipped, - evalErr: enginerr.ErrEvaluationSkipSilently, - expected: engif.ActionCmdDoNothing, + name: "eval skipped -> do nothing", + prevStatus: RemediationStatusSkipped, + hasPrev: true, + evalErr: enginerr.ErrEvaluationSkipSilently, + expected: engif.ActionCmdDoNothing, }, { - name: "nil DB row, eval failure -> on", - nilRow: true, + name: "no previous eval, eval failure -> on", + hasPrev: false, evalErr: enginerr.NewErrEvaluationFailed("failed"), expected: engif.ActionCmdOn, }, { - name: "nil DB row, eval success -> do nothing", - nilRow: true, + name: "no previous eval, eval success -> do nothing", + hasPrev: false, evalErr: nil, expected: engif.ActionCmdDoNothing, }, @@ -103,11 +111,12 @@ func TestShouldRemediate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - var prevRow *db.ListRuleEvaluationsByProfileIdRow - if !tt.nilRow { - prevRow = &db.ListRuleEvaluationsByProfileIdRow{RemStatus: tt.prevEval} + var prev *PreviousEval + if tt.hasPrev { + prev = &PreviousEval{RemediationStatus: tt.prevStatus} } - got := shouldRemediate(prevRow, tt.evalErr) + status := mapEvalStatus(tt.evalErr) + got := shouldRemediate(prev, status) assert.Equal(t, tt.expected, got) }) } @@ -118,8 +127,8 @@ func TestShouldAlert(t *testing.T) { tests := []struct { name string - prevAlert db.AlertStatusTypes - nilRow bool + prevAlert AlertStatus + hasPrev bool evalErr error remErr error remType string @@ -128,7 +137,8 @@ func TestShouldAlert(t *testing.T) { // Happy path: eval success { name: "eval success, alert on -> off", - prevAlert: db.AlertStatusTypesOn, + prevAlert: AlertStatusOn, + hasPrev: true, evalErr: nil, remErr: nil, // Using pull_request.RemediateType so we reach the switch instead of @@ -138,7 +148,8 @@ func TestShouldAlert(t *testing.T) { }, { name: "eval success, alert already off -> do nothing", - prevAlert: db.AlertStatusTypesOff, + prevAlert: AlertStatusOff, + hasPrev: true, evalErr: nil, remErr: nil, remType: pull_request.RemediateType, @@ -147,7 +158,8 @@ func TestShouldAlert(t *testing.T) { // Happy path: eval failure triggers alert { name: "pr remediation eval failure, alert skipped -> on", - prevAlert: db.AlertStatusTypesSkipped, + prevAlert: AlertStatusSkipped, + hasPrev: true, evalErr: enginerr.NewErrEvaluationFailed("failed"), remErr: nil, remType: pull_request.RemediateType, @@ -155,7 +167,8 @@ func TestShouldAlert(t *testing.T) { }, { name: "pr remediation eval failure, alert already on -> do nothing", - prevAlert: db.AlertStatusTypesOn, + prevAlert: AlertStatusOn, + hasPrev: true, evalErr: enginerr.NewErrEvaluationFailed("failed"), remErr: nil, remType: pull_request.RemediateType, @@ -164,7 +177,8 @@ func TestShouldAlert(t *testing.T) { // Non-PR successful remediation (Case 1 early return) { name: "successful non-pr remediation, alert on -> off", - prevAlert: db.AlertStatusTypesOn, + prevAlert: AlertStatusOn, + hasPrev: true, evalErr: enginerr.NewErrEvaluationFailed("failed"), remErr: nil, remType: "some-other-type", @@ -172,7 +186,8 @@ func TestShouldAlert(t *testing.T) { }, { name: "successful non-pr remediation, alert already off -> do nothing", - prevAlert: db.AlertStatusTypesOff, + prevAlert: AlertStatusOff, + hasPrev: true, evalErr: enginerr.NewErrEvaluationFailed("failed"), remErr: nil, remType: "some-other-type", @@ -181,7 +196,8 @@ func TestShouldAlert(t *testing.T) { // Expected errors { name: "eval error -> do nothing", - prevAlert: db.AlertStatusTypesOff, + prevAlert: AlertStatusOff, + hasPrev: true, evalErr: errors.New("generic error"), remErr: nil, remType: pull_request.RemediateType, @@ -189,16 +205,16 @@ func TestShouldAlert(t *testing.T) { }, // Edge cases { - name: "nil DB row, eval failure -> on", - nilRow: true, + name: "no previous eval, eval failure -> on", + hasPrev: false, evalErr: enginerr.NewErrEvaluationFailed("failed"), remErr: nil, remType: pull_request.RemediateType, expected: engif.ActionCmdOn, }, { - name: "nil DB row, eval success -> off", - nilRow: true, + name: "no previous eval, eval success -> off", + hasPrev: false, evalErr: nil, remErr: nil, remType: pull_request.RemediateType, @@ -209,11 +225,12 @@ func TestShouldAlert(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - var prevRow *db.ListRuleEvaluationsByProfileIdRow - if !tt.nilRow { - prevRow = &db.ListRuleEvaluationsByProfileIdRow{AlertStatus: tt.prevAlert} + var prev *PreviousEval + if tt.hasPrev { + prev = &PreviousEval{AlertStatus: tt.prevAlert} } - got := shouldAlert(prevRow, tt.evalErr, tt.remErr, tt.remType) + status := mapEvalStatus(tt.evalErr) + got := shouldAlert(prev, status, tt.remErr, tt.remType) assert.Equal(t, tt.expected, got) }) } diff --git a/internal/engine/actions/types.go b/internal/engine/actions/types.go new file mode 100644 index 0000000000..4cd76d5faf --- /dev/null +++ b/internal/engine/actions/types.go @@ -0,0 +1,58 @@ +// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +package actions + +import "encoding/json" + +// RemediationStatus represents a high-level remediation state, decoupled +// from database-specific types. +type RemediationStatus string + +// RemediationStatus* constants mirror the remediation status lifecycle used +// for actions while remaining decoupled from database-specific types. +const ( + RemediationStatusSuccess RemediationStatus = "success" + RemediationStatusFailure RemediationStatus = "failure" + RemediationStatusError RemediationStatus = "error" + RemediationStatusSkipped RemediationStatus = "skipped" + RemediationStatusNotAvailable RemediationStatus = "not_available" + RemediationStatusPending RemediationStatus = "pending" +) + +// AlertStatus represents a high-level alert state, decoupled from +// database-specific types. +type AlertStatus string + +// AlertStatus* constants mirror the alert status lifecycle used for actions +// while remaining decoupled from database-specific types. +const ( + AlertStatusOn AlertStatus = "on" + AlertStatusOff AlertStatus = "off" + AlertStatusError AlertStatus = "error" + AlertStatusSkipped AlertStatus = "skipped" + AlertStatusNotAvailable AlertStatus = "not_available" +) + +// EvalStatus represents the normalized evaluation status derived from +// rule evaluation errors. +type EvalStatus string + +// EvalStatus* constants represent the normalized evaluation status derived +// from rule evaluation errors. +const ( + EvalStatusSuccess EvalStatus = "success" + EvalStatusFailure EvalStatus = "failure" + EvalStatusError EvalStatus = "error" + EvalStatusSkipped EvalStatus = "skipped" + EvalStatusPending EvalStatus = "pending" +) + +// PreviousEval captures the previous remediation and alert state along with +// associated metadata in a database-agnostic form. +type PreviousEval struct { + RemediationStatus RemediationStatus + AlertStatus AlertStatus + RemediationMeta *json.RawMessage + AlertMeta *json.RawMessage +}