Skip to content

Commit

Permalink
🐛 improve logs in parser (#524)
Browse files Browse the repository at this point in the history 
Fixes #517

---------

Signed-off-by: Pranav Gaikwad <[email protected]>
  • Loading branch information
@pranavgaikwad
pranavgaikwad authored Mar 6, 2024
1 parent d483bcb commit 3625e88
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
4 changes: 2 additions & 2 deletions engine/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ func (r *ruleEngine) RunRules(ctx context.Context, ruleSets []RuleSet, selectors
} else if response.ConditionResponse.Matched && len(response.ConditionResponse.Incidents) > 0 {
violation, err := r.createViolation(ctx, response.ConditionResponse, response.Rule)
if err != nil {
r.logger.Error(err, "unable to create violation from response")
r.logger.Error(err, "unable to create violation from response", "ruleID", response.Rule.RuleID)
}
if len(violation.Incidents) == 0 {
r.logger.V(5).Info("rule was evaluated and incidents were filtered out to make it unmatched", "rule", response.Rule.RuleID)
Expand Down Expand Up @@ -333,7 +333,7 @@ func (r *ruleEngine) runTaggingRules(ctx context.Context, infoRules []ruleMessag
}
templateString, err := r.createPerformString(tagString, variables)
if err != nil {
r.logger.Error(err, "unable to create tag string")
r.logger.Error(err, "unable to create tag string", "ruleID", rule.RuleID)
continue
}
tags[templateString] = true
Expand Down
26 changes: 23 additions & 3 deletions parser/rule_parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provider.InternalProviderClient, error) {
content, err := os.ReadFile(filepath)
if err != nil {
r.Log.V(8).Error(err, "filepath", filepath)
return nil, nil, err
}
// Determine if the content has a ruleset header.
Expand All @@ -180,8 +181,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
// Assume that there is a rule set header.
err = yaml.Unmarshal(content, &ruleMap)
if err != nil {
r.Log.V(8).Error(err, fmt.Sprintf("unable to load rule set - failed to convert file: %s to yaml", filepath))

r.Log.V(8).Error(err, "unable to load rule set, failed to convert file to yaml", "file", filepath)
return nil, nil, fmt.Errorf("unable to convert file: %s to yaml", filepath)
}

Expand All @@ -195,14 +195,16 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
for _, ruleMap := range ruleMap {
ruleID, ok := ruleMap["ruleID"].(string)
if !ok {
r.Log.V(8).Info("ruleID not found", "file", filepath)
return nil, nil, fmt.Errorf("unable to find ruleID in rule")
}

if _, ok := ruleIDMap[ruleID]; ok {
r.Log.V(8).Info("duplicate ruleID", "file", filepath, "ruleID", ruleID)
return nil, nil, fmt.Errorf("duplicated rule id: %v", ruleID)
}
if e, ok := validateRuleID(ruleID); !ok {
r.Log.Info("invalid rule", "reason", e)
r.Log.Info("invalid rule", "reason", e, "ruleID", ruleID)
continue
}

Expand All @@ -217,6 +219,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
case "message":
message, ok := val.(string)
if !ok {
r.Log.V(8).Info("message must be a string", "ruleID", ruleID)
return nil, nil, fmt.Errorf("message must be a string")
}

Expand Down Expand Up @@ -248,11 +251,13 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
case "tag":
tagList, ok := val.([]interface{})
if !ok {
r.Log.V(8).Info("tag must be a list of strings", "ruleID", ruleID)
return nil, nil, fmt.Errorf("tag must be a list of strings")
}
for _, tagVal := range tagList {
tag, ok := tagVal.(string)
if !ok {
r.Log.V(8).Info("tag value must be a string", "ruleID", ruleID, "tag", tagVal)
return nil, nil, fmt.Errorf("tag value must be a string")
}
perform.Tag = append(perform.Tag, tag)
Expand All @@ -262,6 +267,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
}

if err := perform.Validate(); err != nil {
r.Log.V(8).Error(err, "failed validating perform", "ruleID", ruleID, "file", filepath)
return nil, nil, err
}

Expand All @@ -276,6 +282,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid

whenMap, ok := ruleMap["when"].(map[interface{}]interface{})
if !ok {
r.Log.V(8).Info("a rule must have a single condition", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("a Rule must have a single condition")
}

Expand All @@ -288,6 +295,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
delete(whenMap, "from")
from, ok = fromRaw.(string)
if !ok {
r.Log.V(8).Info("a rule must have a single condition", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("from must be a string literal, not %v", fromRaw)
}
}
Expand All @@ -296,6 +304,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
delete(whenMap, "as")
as, ok = asRaw.(string)
if !ok {
r.Log.V(8).Info("as must be a string literal", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("as must be a string literal, not %v", asRaw)
}
}
Expand All @@ -304,6 +313,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
delete(whenMap, "ignore")
ignorable, ok = ignorableRaw.(bool)
if !ok {
r.Log.V(8).Info("ignore must be a boolean", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("ignore must be a boolean, not %v", ignorableRaw)
}
}
Expand All @@ -315,6 +325,7 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
delete(whenMap, "not")
not, ok = notKeywordRaw.(bool)
if !ok {
r.Log.V(8).Info("not must be a boolean", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("not must be a boolean, not %v", notKeywordRaw)
}
}
Expand All @@ -323,17 +334,20 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
for k, value := range whenMap {
key, ok := k.(string)
if !ok {
r.Log.V(8).Info("condition key must be a string", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("condition key must be a string")
}
switch key {
case "or":
//Handle when clause
m, ok := value.([]interface{})
if !ok {
r.Log.V(8).Info("invalid type for or clause, must be an array", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("invalid type for or clause, must be an array")
}
conditions, provs, err := r.getConditions(m)
if err != nil {
r.Log.V(8).Error(err, "failed parsing conditions in or clause", "ruleID", ruleID, "file", filepath)
return nil, nil, err
}
if len(conditions) == 0 {
Expand All @@ -357,10 +371,12 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
//Handle when clause
m, ok := value.([]interface{})
if !ok {
r.Log.V(8).Info("invalid type for and clause, must be an array", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("invalid type for and clause, must be an array")
}
conditions, provs, err := r.getConditions(m)
if err != nil {
r.Log.V(8).Error(err, "failed parsing conditions in and clause", "ruleID", ruleID, "file", filepath)
return nil, nil, err
}
if len(conditions) == 0 {
Expand All @@ -380,17 +396,21 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
}
}
case "":
r.Log.V(8).Info("must have at least one condition", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("must have at least one condition")
default:
// Handle provider
s := strings.Split(key, ".")
if len(s) != 2 {
r.Log.V(8).Info("condition must be of the form {provider}.{capability}", "ruleID", ruleID, "file", filepath)
return nil, nil, fmt.Errorf("condition must be of the form {provider}.{capability}")
}
providerKey, capability := s[0], s[1]

condition, provider, err := r.getConditionForProvider(providerKey, capability, value)
if err != nil {
r.Log.V(8).Error(err, "failed parsing conditions for provider",
"provider", providerKey, "capability", capability, "ruleID", ruleID, "file", filepath)
return nil, nil, err
}
if condition == nil {
Expand Down

0 comments on commit 3625e88

Please sign in to comment.