Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use nil evaluation state if there is no previous evaluation #4197

Merged
merged 1 commit into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,6 @@ examples/rules-and-profiles

# Ignore generated sigstore artifacts while running this locally
tuf-repo-cdn.sigstore.dev.json

# Offline tokens from minder CLI
offline.token
4 changes: 2 additions & 2 deletions database/mock/store.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions internal/db/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
type ExtendQuerier interface {
Querier
GetRuleEvaluationByProfileIdAndRuleType(ctx context.Context, profileID uuid.UUID, entityType NullEntities,
ruleName sql.NullString, entityID uuid.NullUUID, ruleTypeName sql.NullString) (ListRuleEvaluationsByProfileIdRow, error)
ruleName sql.NullString, entityID uuid.NullUUID, ruleTypeName sql.NullString) (*ListRuleEvaluationsByProfileIdRow, error)
}

// Store provides all functions to execute db queries and transactions
Expand Down Expand Up @@ -110,7 +110,7 @@ func (q *Queries) GetRuleEvaluationByProfileIdAndRuleType(
ruleName sql.NullString,
entityID uuid.NullUUID,
ruleTypeName sql.NullString,
) (ListRuleEvaluationsByProfileIdRow, error) {
) (*ListRuleEvaluationsByProfileIdRow, error) {
params := ListRuleEvaluationsByProfileIdParams{
ProfileID: profileID,
EntityType: entityType,
Expand All @@ -120,17 +120,17 @@ func (q *Queries) GetRuleEvaluationByProfileIdAndRuleType(
}
res, err := q.ListRuleEvaluationsByProfileId(ctx, params)
if err != nil {
return ListRuleEvaluationsByProfileIdRow{}, err
return nil, err
}

// Single or no row expected
switch len(res) {
case 0:
return ListRuleEvaluationsByProfileIdRow{}, nil
return nil, nil
case 1:
return res[0], nil
return &res[0], nil
}
return ListRuleEvaluationsByProfileIdRow{},
return nil,
fmt.Errorf("GetRuleEvaluationByProfileIdAndRuleType - expected 1 row, got %d", len(res))
}

Expand Down
25 changes: 16 additions & 9 deletions internal/engine/actions/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"runtime/debug"

"github.com/rs/zerolog"
"github.com/sqlc-dev/pqtype"
"google.golang.org/protobuf/reflect/protoreflect"

"github.com/stacklok/minder/internal/db"
Expand Down Expand Up @@ -121,7 +120,7 @@ func (rae *RuleActionsEngine) DoActions(
cmd := shouldRemediate(params.GetEvalStatusFromDb(), params.GetEvalErr())
// Run remediation
result.RemediateMeta, result.RemediateErr = rae.processAction(ctx, remediate.ActionType, cmd, ent, params,
getMeta(params.GetEvalStatusFromDb().RemMetadata))
getRemediationMeta(params.GetEvalStatusFromDb()))
}

// Try alerting
Expand All @@ -130,7 +129,7 @@ func (rae *RuleActionsEngine) DoActions(
cmd := shouldAlert(params.GetEvalStatusFromDb(), params.GetEvalErr(), result.RemediateErr, remediateEngine.Type())
// Run alerting
result.AlertMeta, result.AlertErr = rae.processAction(ctx, alert.ActionType, cmd, ent, params,
getMeta(params.GetEvalStatusFromDb().AlertMetadata))
getAlertMeta(params.GetEvalStatusFromDb()))
}
return result
}
Expand Down Expand Up @@ -166,7 +165,7 @@ func shouldRemediate(prevEvalFromDb *db.ListRuleEvaluationsByProfileIdRow, evalE

// Get previous Remediation status
prevRemediation := db.RemediationStatusTypesSkipped
if prevEvalFromDb.RemStatus.Valid {
if prevEvalFromDb != nil && prevEvalFromDb.RemStatus.Valid {
prevRemediation = prevEvalFromDb.RemStatus.RemediationStatusTypes
}

Expand Down Expand Up @@ -215,7 +214,7 @@ func shouldAlert(

// Get previous Alert status
prevAlert := db.AlertStatusTypesSkipped
if prevEvalFromDb.AlertStatus.Valid {
if prevEvalFromDb != nil && prevEvalFromDb.AlertStatus.Valid {
prevAlert = prevEvalFromDb.AlertStatus.AlertStatusTypes
}

Expand Down Expand Up @@ -299,10 +298,18 @@ func (rae *RuleActionsEngine) isSkippable(ctx context.Context, actionType engif.
return skipAction
}

// getMeta returns the json.RawMessage from the database type, empty if not valid
func getMeta(rawMsg pqtype.NullRawMessage) *json.RawMessage {
if rawMsg.Valid {
return &rawMsg.RawMessage
// getRemediationMeta returns the json.RawMessage from the database type, empty if not valid
func getRemediationMeta(prevEvalFromDb *db.ListRuleEvaluationsByProfileIdRow) *json.RawMessage {
if prevEvalFromDb != nil && prevEvalFromDb.RemMetadata.Valid {
return &prevEvalFromDb.RemMetadata.RawMessage
}
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 && prevEvalFromDb.AlertMetadata.Valid {
return &prevEvalFromDb.AlertMetadata.RawMessage
}
return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,9 @@ func (_ *Alert) runDoNothing(ctx context.Context, params *paramsSA) (json.RawMes
logger.Debug().Msg("Running do nothing")

// Return the previous alert status.
err := enginerr.AlertStatusAsError(params.prevStatus.AlertStatus.AlertStatusTypes)
err := enginerr.AlertStatusAsError(params.prevStatus)
// If there is a valid alert metadata, return it too
if params.prevStatus.AlertMetadata.Valid {
if params.prevStatus != nil && params.prevStatus.AlertMetadata.Valid {
return params.prevStatus.AlertMetadata.RawMessage, err
}
// If there is no alert metadata, return nil as the metadata and the error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,9 +580,9 @@ func (_ *Remediator) runDoNothing(ctx context.Context, p *paramsPR) (json.RawMes
logger.Debug().Msg("Running do nothing")

// Return the previous remediation status.
err := enginerr.RemediationStatusAsError(p.prevStatus.RemStatus)
err := enginerr.RemediationStatusAsError(p.prevStatus)
// If there is a valid remediation metadata, return it too
if p.prevStatus.RemMetadata.Valid {
if p.prevStatus != nil && p.prevStatus.RemMetadata.Valid {
return p.prevStatus.RemMetadata.RawMessage, err
}
// If there is no remediation metadata, return nil as the metadata and the error
Expand Down
14 changes: 10 additions & 4 deletions internal/engine/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,12 @@ func ErrorAsRemediationStatus(err error) db.RemediationStatusTypes {
}

// RemediationStatusAsError returns the remediation status for a given error
func RemediationStatusAsError(ns db.NullRemediationStatusTypes) error {
if !ns.Valid {
func RemediationStatusAsError(prevStatus *db.ListRuleEvaluationsByProfileIdRow) error {
if prevStatus == nil || !prevStatus.RemStatus.Valid {
return ErrActionSkipped
}

s := ns.RemediationStatusTypes
s := prevStatus.RemStatus.RemediationStatusTypes
switch s {
case db.RemediationStatusTypesSuccess:
return nil
Expand Down Expand Up @@ -203,7 +203,13 @@ func ErrorAsAlertStatus(err error) db.AlertStatusTypes {
}

// AlertStatusAsError returns the error for a given alert status
func AlertStatusAsError(s db.AlertStatusTypes) error {
func AlertStatusAsError(prevStatus *db.ListRuleEvaluationsByProfileIdRow) error {
if prevStatus == nil || !prevStatus.AlertStatus.Valid {
return errors.New("no previous alert state")
}

s := prevStatus.AlertStatus.AlertStatusTypes

switch s {
case db.AlertStatusTypesOn:
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/eval_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (e *executor) createEvalStatusParams(
}

// Save the current rule evaluation status to the evalParams
params.EvalStatusFromDb = &evalStatus
params.EvalStatusFromDb = evalStatus

return params, nil
}
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestExecutor_handleEntityEvent(t *testing.T) {
gomock.Any(),
gomock.Any(),
gomock.Any(),
).Return(db.ListRuleEvaluationsByProfileIdRow{}, nil)
).Return(nil, nil)

mockStore.EXPECT().
GetProviderByID(gomock.Any(), gomock.Eq(providerID)).
Expand Down
1 change: 1 addition & 0 deletions internal/engine/interfaces/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ func (e *EvalStatusParams) GetRule() *models.RuleInstance {
}

// GetEvalStatusFromDb returns the evaluation status from the database
// Returns nil if there is no previous state for this rule/entity
func (e *EvalStatusParams) GetEvalStatusFromDb() *db.ListRuleEvaluationsByProfileIdRow {
return e.EvalStatusFromDb
}
Expand Down
Loading