feat: add concurrency option to parallelize package loading (#778)

* feat: add concurrency option to parallelize package loading

* refactor: move wg.add inside the for loop

* fix: gracefully stop the workers on error

* test: add test for concurrent scan
This commit is contained in:
kruskal 2022-02-16 18:23:37 +01:00 committed by GitHub
parent 43577cebb7
commit 7d539ed494
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 96 additions and 17 deletions

View file

@ -29,6 +29,7 @@ import (
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
"sync"
"golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages"
) )
@ -88,6 +89,7 @@ type Analyzer struct {
excludeGenerated bool excludeGenerated bool
showIgnored bool showIgnored bool
trackSuppressions bool trackSuppressions bool
concurrency int
} }
// SuppressionInfo object is to record the kind and the justification that used // SuppressionInfo object is to record the kind and the justification that used
@ -98,7 +100,7 @@ type SuppressionInfo struct {
} }
// NewAnalyzer builds a new analyzer. // NewAnalyzer builds a new analyzer.
func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressions bool, logger *log.Logger) *Analyzer { func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressions bool, concurrency int, logger *log.Logger) *Analyzer {
ignoreNoSec := false ignoreNoSec := false
if enabled, err := conf.IsGlobalEnabled(Nosec); err == nil { if enabled, err := conf.IsGlobalEnabled(Nosec); err == nil {
ignoreNoSec = enabled ignoreNoSec = enabled
@ -121,6 +123,7 @@ func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressio
stats: &Metrics{}, stats: &Metrics{},
errors: make(map[string][]Error), errors: make(map[string][]Error),
tests: tests, tests: tests,
concurrency: concurrency,
excludeGenerated: excludeGenerated, excludeGenerated: excludeGenerated,
trackSuppressions: trackSuppressions, trackSuppressions: trackSuppressions,
} }
@ -153,15 +156,64 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error
Tests: gosec.tests, Tests: gosec.tests,
} }
for _, pkgPath := range packagePaths { type result struct {
pkgs, err := gosec.load(pkgPath, config) pkgPath string
if err != nil { pkgs []*packages.Package
gosec.AppendError(pkgPath, err) err error
} }
for _, pkg := range pkgs {
results := make(chan result)
jobs := make(chan string, len(packagePaths))
quit := make(chan struct{})
var wg sync.WaitGroup
worker := func(j chan string, r chan result, quit chan struct{}) {
for {
select {
case s := <-j:
packages, err := gosec.load(s, config)
select {
case r <- result{pkgPath: s, pkgs: packages, err: err}:
case <-quit:
// we've been told to stop, probably an error while
// processing a previous result.
wg.Done()
return
}
default:
// j is empty and there are no jobs left
wg.Done()
return
}
}
}
// fill the buffer
for _, pkgPath := range packagePaths {
jobs <- pkgPath
}
for i := 0; i < gosec.concurrency; i++ {
wg.Add(1)
go worker(jobs, results, quit)
}
go func() {
wg.Wait()
close(results)
}()
for r := range results {
if r.err != nil {
gosec.AppendError(r.pkgPath, r.err)
}
for _, pkg := range r.pkgs {
if pkg.Name != "" { if pkg.Name != "" {
err := gosec.ParseErrors(pkg) err := gosec.ParseErrors(pkg)
if err != nil { if err != nil {
close(quit)
wg.Wait() // wait for the goroutines to stop
return fmt.Errorf("parsing errors in pkg %q: %w", pkg.Name, err) return fmt.Errorf("parsing errors in pkg %q: %w", pkg.Name, err)
} }
gosec.Check(pkg) gosec.Check(pkg)

View file

@ -24,7 +24,7 @@ var _ = Describe("Analyzer", func() {
) )
BeforeEach(func() { BeforeEach(func() {
logger, _ = testutils.NewLogger() logger, _ = testutils.NewLogger()
analyzer = gosec.NewAnalyzer(nil, tests, false, false, logger) analyzer = gosec.NewAnalyzer(nil, tests, false, false, 1, logger)
}) })
Context("when processing a package", func() { Context("when processing a package", func() {
@ -77,6 +77,29 @@ var _ = Describe("Analyzer", func() {
Expect(metrics.NumFiles).To(Equal(2)) Expect(metrics.NumFiles).To(Equal(2))
}) })
It("should be able to analyze multiple Go files concurrently", func() {
customAnalyzer := gosec.NewAnalyzer(nil, true, true, false, 32, logger)
customAnalyzer.LoadRules(rules.Generate(false).RulesInfo())
pkg := testutils.NewTestPackage()
defer pkg.Close()
pkg.AddFile("foo.go", `
package main
func main(){
bar()
}`)
pkg.AddFile("bar.go", `
package main
func bar(){
println("package has two files!")
}`)
err := pkg.Build()
Expect(err).ShouldNot(HaveOccurred())
err = customAnalyzer.Process(buildTags, pkg.Path)
Expect(err).ShouldNot(HaveOccurred())
_, metrics, _ := customAnalyzer.Report()
Expect(metrics.NumFiles).To(Equal(2))
})
It("should be able to analyze multiple Go packages", func() { It("should be able to analyze multiple Go packages", func() {
analyzer.LoadRules(rules.Generate(false).RulesInfo()) analyzer.LoadRules(rules.Generate(false).RulesInfo())
pkg1 := testutils.NewTestPackage() pkg1 := testutils.NewTestPackage()
@ -262,7 +285,7 @@ var _ = Describe("Analyzer", func() {
// overwrite nosec option // overwrite nosec option
nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig := gosec.NewConfig()
nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true") nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true")
customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
nosecPackage := testutils.NewTestPackage() nosecPackage := testutils.NewTestPackage()
@ -286,7 +309,7 @@ var _ = Describe("Analyzer", func() {
nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig := gosec.NewConfig()
nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true") nosecIgnoreConfig.SetGlobal(gosec.Nosec, "true")
nosecIgnoreConfig.SetGlobal(gosec.ShowIgnored, "true") nosecIgnoreConfig.SetGlobal(gosec.ShowIgnored, "true")
customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
nosecPackage := testutils.NewTestPackage() nosecPackage := testutils.NewTestPackage()
@ -379,7 +402,7 @@ var _ = Describe("Analyzer", func() {
// overwrite nosec option // overwrite nosec option
nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig := gosec.NewConfig()
nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "#falsePositive") nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "#falsePositive")
customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
nosecPackage := testutils.NewTestPackage() nosecPackage := testutils.NewTestPackage()
@ -402,7 +425,7 @@ var _ = Describe("Analyzer", func() {
// overwrite nosec option // overwrite nosec option
nosecIgnoreConfig := gosec.NewConfig() nosecIgnoreConfig := gosec.NewConfig()
nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "#falsePositive") nosecIgnoreConfig.SetGlobal(gosec.NoSecAlternative, "#falsePositive")
customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, logger) customAnalyzer := gosec.NewAnalyzer(nosecIgnoreConfig, tests, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false, rules.NewRuleFilter(false, "G401")).RulesInfo())
nosecPackage := testutils.NewTestPackage() nosecPackage := testutils.NewTestPackage()
@ -418,7 +441,7 @@ var _ = Describe("Analyzer", func() {
}) })
It("should be able to analyze Go test package", func() { It("should be able to analyze Go test package", func() {
customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, logger) customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false).RulesInfo())
pkg := testutils.NewTestPackage() pkg := testutils.NewTestPackage()
defer pkg.Close() defer pkg.Close()
@ -443,7 +466,7 @@ var _ = Describe("Analyzer", func() {
Expect(issues).Should(HaveLen(1)) Expect(issues).Should(HaveLen(1))
}) })
It("should be able to scan generated files if NOT excluded", func() { It("should be able to scan generated files if NOT excluded", func() {
customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, logger) customAnalyzer := gosec.NewAnalyzer(nil, true, false, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false).RulesInfo())
pkg := testutils.NewTestPackage() pkg := testutils.NewTestPackage()
defer pkg.Close() defer pkg.Close()
@ -464,7 +487,7 @@ var _ = Describe("Analyzer", func() {
Expect(issues).Should(HaveLen(1)) Expect(issues).Should(HaveLen(1))
}) })
It("should be able to skip generated files if excluded", func() { It("should be able to skip generated files if excluded", func() {
customAnalyzer := gosec.NewAnalyzer(nil, true, true, false, logger) customAnalyzer := gosec.NewAnalyzer(nil, true, true, false, 1, logger)
customAnalyzer.LoadRules(rules.Generate(false).RulesInfo()) customAnalyzer.LoadRules(rules.Generate(false).RulesInfo())
pkg := testutils.NewTestPackage() pkg := testutils.NewTestPackage()
defer pkg.Close() defer pkg.Close()
@ -671,7 +694,7 @@ var _ = Describe("Analyzer", func() {
Context("when tracking suppressions", func() { Context("when tracking suppressions", func() {
BeforeEach(func() { BeforeEach(func() {
analyzer = gosec.NewAnalyzer(nil, tests, false, true, logger) analyzer = gosec.NewAnalyzer(nil, tests, false, true, 1, logger)
}) })
It("should not report an error if the violation is suppressed", func() { It("should not report an error if the violation is suppressed", func() {

View file

@ -20,6 +20,7 @@ import (
"io/ioutil" "io/ioutil"
"log" "log"
"os" "os"
"runtime"
"sort" "sort"
"strings" "strings"
@ -114,6 +115,9 @@ var (
// fail by confidence // fail by confidence
flagConfidence = flag.String("confidence", "low", "Filter out the issues with a lower confidence than the given value. Valid options are: low, medium, high") flagConfidence = flag.String("confidence", "low", "Filter out the issues with a lower confidence than the given value. Valid options are: low, medium, high")
// concurrency value
flagConcurrency = flag.Int("concurrency", runtime.NumCPU(), "Concurrency value")
// do not fail // do not fail
flagNoFail = flag.Bool("no-fail", false, "Do not fail the scanning, even if issues were found") flagNoFail = flag.Bool("no-fail", false, "Do not fail the scanning, even if issues were found")
@ -371,7 +375,7 @@ func main() {
} }
// Create the analyzer // Create the analyzer
analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, *flagTrackSuppressions, logger) analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, *flagTrackSuppressions, *flagConcurrency, logger)
analyzer.LoadRules(ruleList.RulesInfo()) analyzer.LoadRules(ruleList.RulesInfo())
excludedDirs := gosec.ExcludedDirsRegExp(flagDirsExclude) excludedDirs := gosec.ExcludedDirsRegExp(flagDirsExclude)

View file

@ -24,7 +24,7 @@ var _ = Describe("gosec rules", func() {
BeforeEach(func() { BeforeEach(func() {
logger, _ = testutils.NewLogger() logger, _ = testutils.NewLogger()
config = gosec.NewConfig() config = gosec.NewConfig()
analyzer = gosec.NewAnalyzer(config, tests, false, false, logger) analyzer = gosec.NewAnalyzer(config, tests, false, false, 1, logger)
runner = func(rule string, samples []testutils.CodeSample) { runner = func(rule string, samples []testutils.CodeSample) {
for n, sample := range samples { for n, sample := range samples {
analyzer.Reset() analyzer.Reset()