-
Notifications
You must be signed in to change notification settings - Fork 275
refactor(vulnfeeds): make nvd conversion run in parallel #4662
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
base: master
Are you sure you want to change the base?
Conversation
…VE for better clarity on which CVE format is in use
| var err error | ||
| switch *outFormat { | ||
| case "OSV": | ||
| err = nvd.CVEToOSV(cve, repos, RepoTagsCache, *outDir, metrics) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: RepoTagsCache and VPRepoCache should be passed in via arguments
| if err != nil { | ||
| return err | ||
| } | ||
| var RepoTagsCache = &git.RepoTagsCache{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these should be defined in the main function and passed through arguments.
| // The RepoTags() call above will have cached the Tag map already | ||
| tagsMap := repoTagsCache[repoURL].Tag | ||
| repoTagsCache[repoURL] = RepoTagsMap{Tag: tagsMap, NormalizedTag: normalizedTags} | ||
| tagsMap, _ := repoTagsCache.Get(repoURL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this probably shouldn't be called tagsMap, since it matches tagsMap on line 241, which is a completely different type, making it a bit confusing.
| }, | ||
| } | ||
| cache := make(RepoTagsCache) | ||
| cache := &RepoTagsCache{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to repoTagsCache to keep it consistent.
|
|
||
| // Adds the repo to the cache for the Vendor/Product combination if not already present. | ||
| func MaybeUpdateVPRepoCache(cache VendorProductToRepoMap, vp *VendorProduct, repo string) { | ||
| func MaybeUpdateVPRepoCache(cache *VPRepoCache, vp *VendorProduct, repo string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These all should be methods on VPRepoCache I think.
| Product string | ||
| } | ||
| type VendorProductToRepoMap map[VendorProduct][]string | ||
| type VPRepoCache struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add an init function as well, and use that instead of manually setting M, and make M private.
This should dramatically improve conversion time.
Also changes NVD conversion to write out ConversionMetrics in the same way that the CVE5 conversion does.
Should be merged after #4650
Confirmed writing out PackageInfo still works too.