ROX-12238: Isolate O.S. matchers to re-use by node scanning #904
Conversation
jvdm
force-pushed
the
jvdm/ROX-12238/1/pull
branch
from
6f6fbcc to
88479b1
Compare
|
Images are ready for the commit at 83625f1. To use the images, use the tag |
jvdm
force-pushed
the
jvdm/ROX-12238/1/pull
branch
from
88479b1 to
a0e0142
Compare
jvdm
changed the title
pkg/matcher/matcher.go
Outdated
| Matcher | ||
|
|
||
| // GetCommonPrefixDirs returns a list with common directory prefixes from all | ||
| // file prefixes in this matcher, but excluding the root directory. The returned |
There was a problem hiding this comment.
Can you elaborate how the common directory prefixes are selected?
|
|
||
| instance = matcher.NewOrMatcher(allMatchers...) | ||
| // SingletonMatcher returns the singleton matcher instance to use for extracting | ||
| // files for analyzing image container. It includes matching for O.S. features |
There was a problem hiding this comment.
nit: It is more common to use OS instead.
pkg/matcher/matcher.go
Outdated
| @@ -39,6 +66,33 @@ func (w *allowlistMatcher) Match(fullPath string, _ os.FileInfo, _ io.ReaderAt) | |||
| return false, false | |||
| } | |||
|
|
|||
| func (w *allowlistMatcher) GetCommonPrefixDirs() []string { | |||
There was a problem hiding this comment.
nit: w *allowlistMatcher, w should be a or m?
pkg/matcher/matcher.go
Outdated
| } | ||
| } | ||
| dirs := pre[""].AsSlice() | ||
| ret := dirs[:0] |
There was a problem hiding this comment.
It looks like the common dir prefix just steps one level down from the root directory and it returns exactly one common prefix per first level dir referenced. And yes, that could be a good balance for how common the common prefix could be. Can you add more comments?
You can also rename the dirs as firstLevelDirs or dirsUnderRoot to make it more clear.
pkg/matcher/matcher.go
Outdated
| } | ||
|
|
||
| func findCommonDirPrefixes(prefixes []string) []string { | ||
| pre := make(map[string]set.StringSet) |
There was a problem hiding this comment.
I want to clarify that the method is doing a search in a trie-like structure finding paths with single-children nodes. Do you think replacing find with search would be better, or do you still think a "map" verb in the name would be clearer?
There was a problem hiding this comment.
I was proposing the alt name for the pre. I am ok with both names for the functions.
There was a problem hiding this comment.
Gotcha. Let me update it, but I will remove the map suffix, since golang is typed, and decorating the name with its type, in this case, is redundant.
jvdm
force-pushed
the
jvdm/ROX-12238/1/pull
branch
from
a0e0142 to
41b9775
Compare
|
/retest |
jvdm
force-pushed
the
jvdm/ROX-12238/1/pull
branch
from
41b9775 to
c257e53
Compare
|
/test slim-e2e-tests |
jvdm
force-pushed
the
jvdm/ROX-12238/1/pull
branch
from
c257e53 to
83625f1
Compare
Create singletons for two different types of "matcher" (O.S. features and active vulnerability), and add a new matcher interface called
PrefixMatcherthat offers a method to retrieve the list of directories to look for files.Node scanning will exclusively use the O.S. matcher. And the list of directories will be used by the node scanner to determine which directories to walk and extract files from. This allows node scanning to be specific on the directories to walk and ignore the other files that are irrelevant during the extraction. Notice that, some analyzers (also known as detectors) do not have a deterministic set of files to scan, they look at prefixes, we still need to walk in directories.
Tests
UT and CI