Skip to content
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

feat: add concurrency option to parallelize package loading #778

Merged
merged 4 commits into from Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 57 additions & 5 deletions analyzer.go
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() {
ccojocar marked this conversation as resolved.
Show resolved Hide resolved
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)
kruskall marked this conversation as resolved.
Show resolved Hide resolved
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
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
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
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