From 7d539ed49444b2c6fd83329677ba6a9b13ecb688 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Wed, 16 Feb 2022 18:23:37 +0100 Subject: [PATCH] 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 --- analyzer.go | 62 +++++++++++++++++++++++++++++++++++++++++---- analyzer_test.go | 41 +++++++++++++++++++++++------- cmd/gosec/main.go | 6 ++++- rules/rules_test.go | 2 +- 4 files changed, 95 insertions(+), 16 deletions(-) diff --git a/analyzer.go b/analyzer.go index e696e3de56..0f9fef2d12 100644 --- a/analyzer.go +++ b/analyzer.go @@ -29,6 +29,7 @@ import ( "regexp" "strconv" "strings" + "sync" "golang.org/x/tools/go/packages" ) @@ -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 @@ -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 @@ -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, } @@ -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) diff --git a/analyzer_test.go b/analyzer_test.go index fdb51bd7ba..8ffcc13086 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -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() { @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() @@ -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() { diff --git a/cmd/gosec/main.go b/cmd/gosec/main.go index 8b84f37d76..9d70558761 100644 --- a/cmd/gosec/main.go +++ b/cmd/gosec/main.go @@ -20,6 +20,7 @@ import ( "io/ioutil" "log" "os" + "runtime" "sort" "strings" @@ -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") @@ -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) diff --git a/rules/rules_test.go b/rules/rules_test.go index 19890cfd16..8cce9bc8f5 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -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()