Skip to content

Commit 11c55ad

Browse files
shawn-hurleyshawn-hurley
committed
Adding test case to handle folder of rules/ruleset.yaml and dir's
Signed-off-by: Shawn Hurley <shawn@hurley.page>
1 parent 942fca1 commit 11c55ad

File tree

10 files changed

+187
-60
lines changed

10 files changed

+187
-60
lines changed

‎engine/engine.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -577,22 +577,20 @@ func (r *ruleEngine) getRelativePathForViolation(fileURI uri.URI) (uri.URI, erro
577577
file := fileURI.Filename()
578578
// get the correct source
579579
for _, locationPrefix := range r.locationPrefixes {
580-
r.logger.Info("filepath-loc-prefix", "file", file, "prefix", locationPrefix)
581580
if strings.Contains(file, locationPrefix) {
582581
sourceLocation = locationPrefix
583582
break
584583
}
585584
}
586-
r.logger.Info("sourceLocation", "sourceLocation", sourceLocation)
587585
absPath, err := filepath.Abs(sourceLocation)
588586
if err != nil {
589587
return fileURI, nil
590588
}
591-
r.logger.Info("sourceLocation", "sourceLocation", sourceLocation, "absPath", absPath)
592589
// given a relative path for source
593590
if absPath != sourceLocation {
594591
relPath := filepath.Join(sourceLocation, strings.TrimPrefix(file, absPath))
595592
newURI := fmt.Sprintf("file:///%s", filepath.Join(strings.TrimPrefix(relPath, "/")))
593+
r.logger.V(7).Info("making filepath relative to source location", "original", fileURI, "new", newURI)
596594
return uri.URI(newURI), nil
597595
}
598596
}

‎parser/rule_parser.go‎

Lines changed: 34 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package parser
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"maps"
78
"os"
@@ -37,21 +38,6 @@ func (e MissingProviderError) Error() string {
3738
return fmt.Sprintf("unable to find provider for: %v", e.Provider)
3839
}
3940

40-
type parserErrors struct {
41-
errs []error
42-
}
43-
44-
func (e parserErrors) Error() string {
45-
s := ""
46-
for i, e := range e.errs {
47-
if i == 0 {
48-
s = e.Error()
49-
}
50-
s = fmt.Sprintf("%s\n%s", s, e.Error())
51-
}
52-
return s
53-
}
54-
5541
type ruleParseReturn struct {
5642
rules []engine.Rule
5743
conditionsByCap map[string][]provider.ConditionsByCap
@@ -72,15 +58,15 @@ func (r *RuleParser) loadRuleSet(dir string) *engine.RuleSet {
7258
info, err := os.Stat(goldenFile)
7359
if err != nil {
7460
r.Log.V(8).Error(err, "unable to load rule set")
75-
return nil
61+
return defaultRuleSet
7662
}
7763
if !info.Mode().IsRegular() {
78-
return nil
64+
return defaultRuleSet
7965
}
8066
content, err := os.ReadFile(goldenFile)
8167
if err != nil {
8268
r.Log.V(8).Error(err, "unable to load rule set")
83-
return nil
69+
return defaultRuleSet
8470
}
8571

8672
set := engine.RuleSet{}
@@ -111,17 +97,12 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
11197

11298
// If a single file, then it must have the ruleset metadata.
11399
if info.Mode().IsRegular() {
114-
rules, m, provConditions, err := r.LoadRule(filepath)
100+
ruleSet := r.loadRuleSet(path.Dir(filepath))
101+
rules, m, provConditions, err := r.loadRule(filepath, ruleSet)
115102
if err != nil {
116103
r.Log.V(8).Error(err, "unable to load rule set")
117104
return nil, nil, nil, err
118105
}
119-
120-
ruleSet := r.loadRuleSet(path.Dir(filepath))
121-
// if nil, use the default rule set
122-
if ruleSet == nil {
123-
ruleSet = defaultRuleSet
124-
}
125106
ruleSet.Rules = rules
126107

127108
return []engine.RuleSet{*ruleSet}, m, provConditions, err
@@ -136,21 +117,21 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
136117
}
137118
var ruleSet *engine.RuleSet
138119
rules := []engine.Rule{}
139-
parserErr := &parserErrors{}
120+
var parseErr error
140121
ruleParserWG := sync.WaitGroup{}
141122
ruleLoadChan := make(chan ruleParseReturn, 10)
142123

143-
loadCtx, _ := context.WithTimeout(context.Background(), 5*time.Minute)
124+
loadCtx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute)
144125
for _, f := range files {
145126
info, err := os.Stat(path.Join(filepath, f.Name()))
146127
if err != nil {
147-
parserErr.errs = append(parserErr.errs, err)
128+
parseErr = errors.Join(parseErr, err)
148129
continue
149130
}
150131
if info.IsDir() {
151132
r, m, provConditions, err := r.LoadRules(path.Join(filepath, f.Name()))
152133
if err != nil {
153-
parserErr.errs = append(parserErr.errs, err)
134+
parseErr = errors.Join(parseErr, err)
154135
continue
155136
}
156137
ruleSets = append(ruleSets, r...)
@@ -168,8 +149,11 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
168149
continue
169150
}
170151
if info.Mode().IsRegular() {
171-
if ruleSet == nil && f.Name() == RULE_SET_GOLDEN_FILE_NAME {
152+
// If we find a file, in a directory we need to load the ruleset
153+
if ruleSet == nil {
172154
ruleSet = r.loadRuleSet(filepath)
155+
}
156+
if f.Name() == RULE_SET_GOLDEN_FILE_NAME {
173157
continue
174158
}
175159
// skip rule tests
@@ -180,7 +164,7 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
180164
}
181165
ruleParserWG.Add(1)
182166
go func() {
183-
rules, m, provConditions, err := r.LoadRule(path.Join(filepath, f.Name()))
167+
rules, m, provConditions, err := r.loadRule(path.Join(filepath, f.Name()), ruleSet)
184168
select {
185169
case ruleLoadChan <- ruleParseReturn{
186170
rules: rules,
@@ -211,11 +195,16 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
211195
continue
212196
}
213197
if load.err != nil {
214-
parserErr.errs = append(parserErr.errs, load.err)
198+
parseErr = errors.Join(parseErr, load.err)
199+
ruleParserWG.Done()
200+
continue
201+
}
202+
if len(load.rules) == 0 {
215203
ruleParserWG.Done()
216204
continue
217205
}
218206
if load.providerMap != nil {
207+
r.Log.Info("providers", "current", clientMap, "toAdd", load.providerMap)
219208
maps.Copy(clientMap, load.providerMap)
220209
}
221210
for k, v := range load.conditionsByCap {
@@ -242,21 +231,19 @@ func (r *RuleParser) LoadRules(filepath string) ([]engine.RuleSet, map[string]pr
242231
}
243232
if ok, err := r.Selector.Matches(meta); !ok && err == nil {
244233
r.Log.V(6).Info("ruleset does not have any rules that match selector, filtering out", "ruleSet", ruleSet.Name)
245-
return nil, nil, nil, nil
234+
cancelFunc()
235+
return ruleSets, clientMap, providerConditions, parseErr
246236
}
247237
}
248238

249239
ruleSet.Rules = rules
250240
ruleSets = append(ruleSets, *ruleSet)
251241
}
252-
// Return nil if there are no captured errors
253-
if len(parserErr.errs) == 0 {
254-
return ruleSets, clientMap, providerConditions, nil
255-
}
256-
return ruleSets, clientMap, providerConditions, parserErr
242+
cancelFunc()
243+
return ruleSets, clientMap, providerConditions, parseErr
257244
}
258245

259-
func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provider.InternalProviderClient, map[string][]provider.ConditionsByCap, error) {
246+
func (r *RuleParser) loadRule(filepath string, ruleSet *engine.RuleSet) ([]engine.Rule, map[string]provider.InternalProviderClient, map[string][]provider.ConditionsByCap, error) {
260247
content, err := os.ReadFile(filepath)
261248
if err != nil {
262249
r.Log.V(8).Error(err, "filepath", filepath)
@@ -368,6 +355,14 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid
368355
}
369356

370357
r.addRuleFields(&rule, ruleMap)
358+
if r.Selector != nil {
359+
meta := &engine.RuleMeta{
360+
Labels: append(ruleSet.Labels, rule.Labels...),
361+
}
362+
if ok, err := r.Selector.Matches(meta); !ok && err == nil {
363+
continue
364+
}
365+
}
371366

372367
whenMap, ok := ruleMap["when"].(map[any]any)
373368
if !ok {

‎parser/rule_parser_test.go‎

Lines changed: 125 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,66 @@ func TestLoadRules(t *testing.T) {
606606
{
607607
Name: "multiple-rulesets",
608608
testFileName: "folder-of-rulesets",
609+
providerNameClient: map[string]provider.InternalProviderClient{
610+
"builtin": testProvider{
611+
caps: []provider.Capability{{
612+
Name: "file",
613+
}},
614+
},
615+
"notadded": testProvider{
616+
caps: []provider.Capability{{
617+
Name: "fake",
618+
}},
619+
},
620+
"fake": testProvider{
621+
caps: []provider.Capability{{
622+
Name: "ref",
623+
}},
624+
},
625+
},
626+
ExpectedRuleSet: map[string]engine.RuleSet{
627+
"file-ruleset-a": {
628+
Rules: []engine.Rule{
629+
{
630+
RuleMeta: engine.RuleMeta{
631+
RuleID: "file-001",
632+
Description: "",
633+
Category: &konveyor.Potential,
634+
},
635+
Perform: engine.Perform{Message: engine.Message{Text: &allGoFiles, Links: []konveyor.Link{}}},
636+
When: engine.ConditionEntry{},
637+
},
638+
},
639+
},
640+
"file-ruleset-b": {
641+
Rules: []engine.Rule{
642+
{
643+
RuleMeta: engine.RuleMeta{RuleID: "ref-001",
644+
Description: "",
645+
Category: &konveyor.Potential,
646+
},
647+
Perform: engine.Perform{Message: engine.Message{Text: &allGoFiles, Links: []konveyor.Link{}}},
648+
When: engine.ConditionEntry{},
649+
},
650+
},
651+
},
652+
},
653+
ExpectedProvider: map[string]provider.InternalProviderClient{
654+
"builtin": testProvider{
655+
caps: []provider.Capability{{
656+
Name: "file",
657+
}},
658+
},
659+
"fake": testProvider{
660+
caps: []provider.Capability{{
661+
Name: "ref",
662+
}},
663+
},
664+
},
665+
},
666+
{
667+
Name: "folder of rules ruleset",
668+
testFileName: "folder-of-rules-ruleset",
609669
providerNameClient: map[string]provider.InternalProviderClient{
610670
"builtin": testProvider{
611671
caps: []provider.Capability{{
@@ -619,6 +679,24 @@ func TestLoadRules(t *testing.T) {
619679
},
620680
},
621681
ExpectedRuleSet: map[string]engine.RuleSet{
682+
"folder-of-rules-rulesets": {
683+
Rules: []engine.Rule{
684+
{
685+
RuleMeta: engine.RuleMeta{
686+
RuleID: "file-001",
687+
Effort: &effort,
688+
Category: &konveyor.Potential,
689+
},
690+
Perform: engine.Perform{
691+
Message: engine.Message{
692+
Text: &allGoFiles,
693+
Links: []konveyor.Link{},
694+
},
695+
},
696+
When: engine.ConditionEntry{},
697+
},
698+
},
699+
},
622700
"file-ruleset-a": {
623701
Rules: []engine.Rule{
624702
{
@@ -653,6 +731,45 @@ func TestLoadRules(t *testing.T) {
653731
},
654732
},
655733
},
734+
{
735+
Name: "folder of rules ruleset filter",
736+
testFileName: "folder-of-rules-ruleset",
737+
providerNameClient: map[string]provider.InternalProviderClient{
738+
"builtin": testProvider{
739+
caps: []provider.Capability{{
740+
Name: "file",
741+
}},
742+
},
743+
"notadded": testProvider{
744+
caps: []provider.Capability{{
745+
Name: "fake",
746+
}},
747+
},
748+
},
749+
ExpectedRuleSet: map[string]engine.RuleSet{
750+
"file-ruleset-a": {
751+
Rules: []engine.Rule{
752+
{
753+
RuleMeta: engine.RuleMeta{
754+
RuleID: "file-001",
755+
Description: "",
756+
Category: &konveyor.Potential,
757+
},
758+
Perform: engine.Perform{Message: engine.Message{Text: &allGoFiles, Links: []konveyor.Link{}}},
759+
When: engine.ConditionEntry{},
760+
},
761+
},
762+
},
763+
},
764+
ExpectedProvider: map[string]provider.InternalProviderClient{
765+
"builtin": testProvider{
766+
caps: []provider.Capability{{
767+
Name: "file",
768+
}},
769+
},
770+
},
771+
Selector: "test=filter",
772+
},
656773
{
657774
Name: "handle not-valid category",
658775
testFileName: "invalid-category.yaml",
@@ -871,20 +988,6 @@ func TestLoadRules(t *testing.T) {
871988
},
872989
When: engine.ConditionEntry{},
873990
},
874-
{
875-
RuleMeta: engine.RuleMeta{
876-
RuleID: "file-002",
877-
Description: "",
878-
Category: &konveyor.Potential,
879-
},
880-
Perform: engine.Perform{
881-
Message: engine.Message{
882-
Text: &allGoFiles,
883-
Links: []konveyor.Link{},
884-
},
885-
},
886-
When: engine.ConditionEntry{},
887-
},
888991
},
889992
},
890993
},
@@ -899,6 +1002,11 @@ func TestLoadRules(t *testing.T) {
8991002
Name: "file",
9001003
}},
9011004
},
1005+
"fake": testProvider{
1006+
caps: []provider.Capability{{
1007+
Name: "ref",
1008+
}},
1009+
},
9021010
"notadded": testProvider{
9031011
caps: []provider.Capability{{
9041012
Name: "fake",
@@ -994,7 +1102,7 @@ func TestLoadRules(t *testing.T) {
9941102
var err error
9951103
ruleParser.Selector, err = labels.NewLabelSelector[*engine.RuleMeta](tc.Selector, nil)
9961104
if err != nil {
997-
t.Error("unable to get selector")
1105+
t.Fatalf("unable to get selector: %v", err)
9981106
return
9991107
}
10001108
}
@@ -1026,6 +1134,7 @@ func TestLoadRules(t *testing.T) {
10261134
expectedProvider, ok := tc.ExpectedProvider[k]
10271135
if !ok {
10281136
t.Errorf("could not find provider: %v", k)
1137+
continue
10291138
}
10301139
expectedCaps := expectedProvider.Capabilities()
10311140
if !reflect.DeepEqual(gotCaps, expectedCaps) {
@@ -1053,7 +1162,7 @@ func TestLoadRules(t *testing.T) {
10531162
compareWhens(expectedRule.When, rule.When, t)
10541163
}
10551164
if !foundRule {
1056-
t.Errorf("not have matching rule go: %#v, expected rules: %#v", rule, expectedSet.Rules)
1165+
t.Errorf("not have matching rule go: %#v\nin ruleset: %v\nexpected rules: %#v", rule, ruleSet.Name, expectedSet.Rules)
10571166
}
10581167
}
10591168
// We will test the conditions getter by itself.

0 commit comments

Comments
 (0)