Skip to content

Commit

Permalink
feat: add concurrency option to parallelize package loading (#778)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@kruskall
kruskall committed Feb 16, 2022
1 parent 43577ce commit 7d539ed
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 16 deletions.
62 changes: 57 additions & 5 deletions analyzer.go
View file
Expand Up @@ -29,6 +29,7 @@ import (
"regexp"
"strconv"
"strings"
"sync"

"golang.org/x/tools/go/packages"
)
Expand Down Expand Up @@ -88,6 +89,7 @@ type Analyzer struct {
excludeGenerated bool
showIgnored bool
trackSuppressions bool
concurrency int
}

// SuppressionInfo object is to record the kind and the justification that used
Expand All @@ -98,7 +100,7 @@ type SuppressionInfo struct {
}

// 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
if enabled, err := conf.IsGlobalEnabled(Nosec); err == nil {
ignoreNoSec = enabled
Expand All @@ -121,6 +123,7 @@ func NewAnalyzer(conf Config, tests bool, excludeGenerated bool, trackSuppressio
stats: &Metrics{},
errors: make(map[string][]Error),
tests: tests,
concurrency: concurrency,
excludeGenerated: excludeGenerated,
trackSuppressions: trackSuppressions,
}
Expand Down Expand Up @@ -153,15 +156,64 @@ func (gosec *Analyzer) Process(buildTags []string, packagePaths ...string) error
Tests: gosec.tests,
}

type result struct {
pkgPath string
pkgs []*packages.Package
err error
}

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 {
pkgs, err := gosec.load(pkgPath, config)
if err != nil {
gosec.AppendError(pkgPath, err)
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 pkgs {
for _, pkg := range r.pkgs {
if pkg.Name != "" {
err := gosec.ParseErrors(pkg)
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)
}
gosec.Check(pkg)
Expand Down
41 changes: 32 additions & 9 deletions analyzer_test.go
View file
Expand Up @@ -24,7 +24,7 @@ var _ = Describe("Analyzer", func() {
)
BeforeEach(func() {
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() {
Expand Down Expand Up @@ -77,6 +77,29 @@ var _ = Describe("Analyzer", func() {
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() {
analyzer.LoadRules(rules.Generate(false).RulesInfo())
pkg1 := testutils.NewTestPackage()
Expand Down Expand Up @@ -262,7 +285,7 @@ var _ = Describe("Analyzer", func() {
// overwrite nosec option
nosecIgnoreConfig := gosec.NewConfig()
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())

nosecPackage := testutils.NewTestPackage()
Expand All @@ -286,7 +309,7 @@ var _ = Describe("Analyzer", func() {
nosecIgnoreConfig := gosec.NewConfig()
nosecIgnoreConfig.SetGlobal(gosec.Nosec, "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())

nosecPackage := testutils.NewTestPackage()
Expand Down Expand Up @@ -379,7 +402,7 @@ var _ = Describe("Analyzer", func() {
// overwrite nosec option
nosecIgnoreConfig := gosec.NewConfig()
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())

nosecPackage := testutils.NewTestPackage()
Expand All @@ -402,7 +425,7 @@ var _ = Describe("Analyzer", func() {
// overwrite nosec option
nosecIgnoreConfig := gosec.NewConfig()
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())

nosecPackage := testutils.NewTestPackage()
Expand All @@ -418,7 +441,7 @@ var _ = Describe("Analyzer", 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())
pkg := testutils.NewTestPackage()
defer pkg.Close()
Expand All @@ -443,7 +466,7 @@ var _ = Describe("Analyzer", func() {
Expect(issues).Should(HaveLen(1))
})
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())
pkg := testutils.NewTestPackage()
defer pkg.Close()
Expand All @@ -464,7 +487,7 @@ var _ = Describe("Analyzer", func() {
Expect(issues).Should(HaveLen(1))
})
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())
pkg := testutils.NewTestPackage()
defer pkg.Close()
Expand Down Expand Up @@ -671,7 +694,7 @@ var _ = Describe("Analyzer", func() {

Context("when tracking suppressions", 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() {
Expand Down
6 changes: 5 additions & 1 deletion cmd/gosec/main.go
View file
Expand Up @@ -20,6 +20,7 @@ import (
"io/ioutil"
"log"
"os"
"runtime"
"sort"
"strings"

Expand Down Expand Up @@ -114,6 +115,9 @@ var (
// 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")

// concurrency value
flagConcurrency = flag.Int("concurrency", runtime.NumCPU(), "Concurrency value")

// do not fail
flagNoFail = flag.Bool("no-fail", false, "Do not fail the scanning, even if issues were found")

Expand Down Expand Up @@ -371,7 +375,7 @@ func main() {
}

// Create the analyzer
analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, *flagTrackSuppressions, logger)
analyzer := gosec.NewAnalyzer(config, *flagScanTests, *flagExcludeGenerated, *flagTrackSuppressions, *flagConcurrency, logger)
analyzer.LoadRules(ruleList.RulesInfo())

excludedDirs := gosec.ExcludedDirsRegExp(flagDirsExclude)
Expand Down
2 changes: 1 addition & 1 deletion rules/rules_test.go
View file
Expand Up @@ -24,7 +24,7 @@ var _ = Describe("gosec rules", func() {
BeforeEach(func() {
logger, _ = testutils.NewLogger()
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) {
for n, sample := range samples {
analyzer.Reset()
Expand Down

0 comments on commit 7d539ed

Please sign in to comment.