From af91535edfe6f02a671921c22d2daebfcfa3e98a Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Wed, 3 Apr 2024 01:41:57 +0300 Subject: [PATCH] refactor(terraform): remove metrics collection (#6444) --- .../adapters/terraform/tftestutil/testutil.go | 4 +- pkg/iac/scanners/helm/test/option_test.go | 16 +-- .../scanners/terraform/deterministic_test.go | 6 +- .../scanners/terraform/executor/executor.go | 45 +-------- .../terraform/executor/executor_test.go | 26 +++-- pkg/iac/scanners/terraform/module_test.go | 99 +++++++++++++------ .../scanners/terraform/parser/evaluator.go | 11 +-- pkg/iac/scanners/terraform/parser/parser.go | 41 +------- .../scanners/terraform/performance_test.go | 6 +- pkg/iac/scanners/terraform/scanner.go | 58 ++--------- pkg/iac/scanners/terraform/scanner_test.go | 6 +- pkg/iac/scanners/terraform/setup_test.go | 6 +- 12 files changed, 126 insertions(+), 198 deletions(-) diff --git a/pkg/iac/adapters/terraform/tftestutil/testutil.go b/pkg/iac/adapters/terraform/tftestutil/testutil.go index 5503bfac5b99..57535cf151c5 100644 --- a/pkg/iac/adapters/terraform/tftestutil/testutil.go +++ b/pkg/iac/adapters/terraform/tftestutil/testutil.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/aquasecurity/trivy/internal/testutil" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" + "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/aquasecurity/trivy/pkg/iac/terraform" ) @@ -13,7 +13,7 @@ func CreateModulesFromSource(t *testing.T, source, ext string) terraform.Modules fs := testutil.CreateFS(t, map[string]string{ "source" + ext: source, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) if err := p.ParseFS(context.TODO(), "."); err != nil { t.Fatal(err) } diff --git a/pkg/iac/scanners/helm/test/option_test.go b/pkg/iac/scanners/helm/test/option_test.go index 2ad7efc64008..d16d29039a15 100644 --- a/pkg/iac/scanners/helm/test/option_test.go +++ b/pkg/iac/scanners/helm/test/option_test.go @@ -7,10 +7,10 @@ import ( "strings" "testing" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/helm/parser" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/aquasecurity/trivy/pkg/iac/scanners/helm/parser" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" ) @@ -37,10 +37,10 @@ func Test_helm_parser_with_options_with_values_file(t *testing.T) { var opts []options.ParserOption if test.valuesFile != "" { - opts = append(opts, parser2.OptionWithValuesFile(test.valuesFile)) + opts = append(opts, parser.OptionWithValuesFile(test.valuesFile)) } - helmParser := parser2.New(chartName, opts...) + helmParser := parser.New(chartName, opts...) err := helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") require.NoError(t, err) manifests, err := helmParser.RenderedChartFiles() @@ -87,14 +87,14 @@ func Test_helm_parser_with_options_with_set_value(t *testing.T) { var opts []options.ParserOption if test.valuesFile != "" { - opts = append(opts, parser2.OptionWithValuesFile(test.valuesFile)) + opts = append(opts, parser.OptionWithValuesFile(test.valuesFile)) } if test.values != "" { - opts = append(opts, parser2.OptionWithValues(test.values)) + opts = append(opts, parser.OptionWithValues(test.values)) } - helmParser := parser2.New(chartName, opts...) + helmParser := parser.New(chartName, opts...) err := helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") require.NoError(t, err) manifests, err := helmParser.RenderedChartFiles() @@ -140,10 +140,10 @@ func Test_helm_parser_with_options_with_api_versions(t *testing.T) { var opts []options.ParserOption if len(test.apiVersions) > 0 { - opts = append(opts, parser2.OptionWithAPIVersions(test.apiVersions...)) + opts = append(opts, parser.OptionWithAPIVersions(test.apiVersions...)) } - helmParser := parser2.New(chartName, opts...) + helmParser := parser.New(chartName, opts...) err := helmParser.ParseFS(context.TODO(), os.DirFS(filepath.Join("testdata", chartName)), ".") require.NoError(t, err) manifests, err := helmParser.RenderedChartFiles() diff --git a/pkg/iac/scanners/terraform/deterministic_test.go b/pkg/iac/scanners/terraform/deterministic_test.go index d47161ec0059..258fe5bbbd16 100644 --- a/pkg/iac/scanners/terraform/deterministic_test.go +++ b/pkg/iac/scanners/terraform/deterministic_test.go @@ -7,7 +7,7 @@ import ( "github.com/aquasecurity/trivy/internal/testutil" "github.com/aquasecurity/trivy/pkg/iac/rules" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/executor" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" + "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/stretchr/testify/require" ) @@ -39,12 +39,12 @@ locals { }) for i := 0; i < 100; i++ { - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), ".") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + results, _ := executor.New().Execute(modules) require.Len(t, results.GetFailed(), 2) } } diff --git a/pkg/iac/scanners/terraform/executor/executor.go b/pkg/iac/scanners/terraform/executor/executor.go index a4d15f45a9bb..efc140b89b46 100644 --- a/pkg/iac/scanners/terraform/executor/executor.go +++ b/pkg/iac/scanners/terraform/executor/executor.go @@ -4,7 +4,6 @@ import ( "fmt" "runtime" "sort" - "time" "github.com/zclconf/go-cty/cty" @@ -38,22 +37,6 @@ type Executor struct { frameworks []framework.Framework } -type Metrics struct { - Timings struct { - Adaptation time.Duration - RunningChecks time.Duration - } - Counts struct { - Ignored int - Failed int - Passed int - Critical int - High int - Medium int - Low int - } -} - // New creates a new Executor func New(options ...Option) *Executor { s := &Executor{ @@ -77,14 +60,10 @@ func checkInList(id string, list []string) bool { return false } -func (e *Executor) Execute(modules terraform.Modules) (scan.Results, Metrics, error) { - - var metrics Metrics +func (e *Executor) Execute(modules terraform.Modules) (scan.Results, error) { e.debug.Log("Adapting modules...") - adaptationTime := time.Now() infra := adapter.Adapt(modules) - metrics.Timings.Adaptation = time.Since(adaptationTime) e.debug.Log("Adapted %d module(s) into defsec state data.", len(modules)) threads := runtime.NumCPU() @@ -101,7 +80,6 @@ func (e *Executor) Execute(modules terraform.Modules) (scan.Results, Metrics, er f(infra) } - checksTime := time.Now() registeredRules := rules.GetRegistered(e.frameworks...) e.debug.Log("Initialized %d rule(s).", len(registeredRules)) @@ -109,9 +87,8 @@ func (e *Executor) Execute(modules terraform.Modules) (scan.Results, Metrics, er e.debug.Log("Created pool with %d worker(s) to apply rules.", threads) results, err := pool.Run() if err != nil { - return nil, metrics, err + return nil, err } - metrics.Timings.RunningChecks = time.Since(checksTime) e.debug.Log("Finished applying rules.") if e.enableIgnores { @@ -152,25 +129,9 @@ func (e *Executor) Execute(modules terraform.Modules) (scan.Results, Metrics, er results = e.updateSeverity(results) results = e.filterResults(results) - metrics.Counts.Ignored = len(results.GetIgnored()) - metrics.Counts.Passed = len(results.GetPassed()) - metrics.Counts.Failed = len(results.GetFailed()) - - for _, res := range results.GetFailed() { - switch res.Severity() { - case severity.Critical: - metrics.Counts.Critical++ - case severity.High: - metrics.Counts.High++ - case severity.Medium: - metrics.Counts.Medium++ - case severity.Low: - metrics.Counts.Low++ - } - } e.sortResults(results) - return results, metrics, nil + return results, nil } func (e *Executor) updateSeverity(results []scan.Result) scan.Results { diff --git a/pkg/iac/scanners/terraform/executor/executor_test.go b/pkg/iac/scanners/terraform/executor/executor_test.go index 952803a507f5..c8c3e2af60c5 100644 --- a/pkg/iac/scanners/terraform/executor/executor_test.go +++ b/pkg/iac/scanners/terraform/executor/executor_test.go @@ -8,7 +8,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/providers" "github.com/aquasecurity/trivy/pkg/iac/rules" "github.com/aquasecurity/trivy/pkg/iac/scan" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" + "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/aquasecurity/trivy/pkg/iac/severity" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/stretchr/testify/assert" @@ -47,12 +47,15 @@ resource "problem" "this" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := New().Execute(modules) + + results, err := New().Execute(modules) + assert.Error(t, err) + assert.Equal(t, len(results.GetFailed()), 0) } @@ -69,12 +72,14 @@ resource "problem" "this" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) + modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - _, _, err = New(OptionStopOnErrors(false)).Execute(modules) + + _, err = New(OptionStopOnErrors(false)).Execute(modules) assert.Error(t, err) } @@ -91,12 +96,15 @@ resource "problem" "this" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := New().Execute(modules) + + results, _ := New().Execute(modules) + require.NoError(t, err) + assert.Equal(t, len(results.GetFailed()), 0) } @@ -113,12 +121,12 @@ resource "problem" "this" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - _, _, err = New(OptionStopOnErrors(false)).Execute(modules) + _, err = New(OptionStopOnErrors(false)).Execute(modules) assert.Error(t, err) } diff --git a/pkg/iac/scanners/terraform/module_test.go b/pkg/iac/scanners/terraform/module_test.go index ffed34718156..61b1a0e359f6 100644 --- a/pkg/iac/scanners/terraform/module_test.go +++ b/pkg/iac/scanners/terraform/module_test.go @@ -13,7 +13,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/executor" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" + "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/aquasecurity/trivy/pkg/iac/severity" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/stretchr/testify/require" @@ -88,13 +88,15 @@ resource "problem" "uhoh" { debug := bytes.NewBuffer([]byte{}) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true), options.ParserWithDebug(debug)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true), options.ParserWithDebug(debug)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, err := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") if t.Failed() { fmt.Println(debug.String()) @@ -119,12 +121,15 @@ resource "problem" "uhoh" { `}, ) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -148,12 +153,15 @@ resource "problem" "uhoh" { `}, ) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleNotFound(t, badRule.LongID(), results, "") } @@ -175,12 +183,15 @@ resource "problem" "uhoh" { } `}) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -202,12 +213,15 @@ resource "problem" "uhoh" { } `}) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -238,12 +252,15 @@ resource "problem" "uhoh" { } `}) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -276,12 +293,15 @@ resource "problem" "uhoh" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true), options.ParserWithDebug(os.Stderr)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true), options.ParserWithDebug(os.Stderr)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -331,12 +351,15 @@ resource "problem" "uhoh" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -380,12 +403,15 @@ resource "problem" "uhoh" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -418,12 +444,15 @@ resource "problem" "uhoh" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -473,12 +502,15 @@ resource "problem" "uhoh" { `, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, badRule.LongID(), results, "") } @@ -523,12 +555,15 @@ resource "bad" "thing" { reg := rules.Register(r1) defer rules.Deregister(reg) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleFound(t, r1.LongID(), results, "") } @@ -572,12 +607,15 @@ resource "bad" "thing" { reg := rules.Register(r1) defer rules.Deregister(reg) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleNotFound(t, r1.LongID(), results, "") } @@ -621,12 +659,15 @@ data "aws_iam_policy_document" "policy" { } `}) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) require.NoError(t, err) - results, _, _ := executor.New().Execute(modules) + + results, err := executor.New().Execute(modules) + require.NoError(t, err) + testutil.AssertRuleNotFound(t, iam.CheckEnforceGroupMFA.LongID(), results, "") } diff --git a/pkg/iac/scanners/terraform/parser/evaluator.go b/pkg/iac/scanners/terraform/parser/evaluator.go index e7e3415e1b52..3633d22386a1 100644 --- a/pkg/iac/scanners/terraform/parser/evaluator.go +++ b/pkg/iac/scanners/terraform/parser/evaluator.go @@ -5,7 +5,6 @@ import ( "errors" "io/fs" "reflect" - "time" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/ext/typeexpr" @@ -119,7 +118,7 @@ func (e *evaluator) exportOutputs() cty.Value { return cty.ObjectVal(data) } -func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[string]fs.FS, time.Duration) { +func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[string]fs.FS) { fsKey := types.CreateFSKey(e.filesystem) e.debug.Log("Filesystem key is '%s'", fsKey) @@ -127,10 +126,7 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str fsMap := make(map[string]fs.FS) fsMap[fsKey] = e.filesystem - var parseDuration time.Duration - var lastContext hcl.EvalContext - start := time.Now() e.debug.Log("Starting module evaluation...") for i := 0; i < maxContextIterations; i++ { @@ -154,8 +150,6 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str e.blocks = e.expandBlocks(e.blocks) e.blocks = e.expandBlocks(e.blocks) - parseDuration += time.Since(start) - e.debug.Log("Starting submodule evaluation...") var modules terraform.Modules for _, definition := range e.loadModules(ctx) { @@ -192,9 +186,8 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str } e.debug.Log("Module evaluation complete.") - parseDuration += time.Since(start) rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) - return append(terraform.Modules{rootModule}, modules...), fsMap, parseDuration + return append(terraform.Modules{rootModule}, modules...), fsMap } func (e *evaluator) expandBlocks(blocks terraform.Blocks) terraform.Blocks { diff --git a/pkg/iac/scanners/terraform/parser/parser.go b/pkg/iac/scanners/terraform/parser/parser.go index 35a8b454f00f..b80c4a6babf2 100644 --- a/pkg/iac/scanners/terraform/parser/parser.go +++ b/pkg/iac/scanners/terraform/parser/parser.go @@ -9,7 +9,6 @@ import ( "path/filepath" "sort" "strings" - "time" "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclparse" @@ -28,19 +27,6 @@ type sourceFile struct { path string } -type Metrics struct { - Timings struct { - DiskIODuration time.Duration - ParseDuration time.Duration - } - Counts struct { - Blocks int - Modules int - ModuleDownloads int - Files int - } -} - var _ ConfigurableTerraformParser = (*Parser)(nil) // Parser is a tool for parsing terraform templates at a given file system location @@ -57,7 +43,6 @@ type Parser struct { workspaceName string underlying *hclparse.Parser children []*Parser - metrics Metrics options []options.ParserOption debug debug.Logger allowDownloads bool @@ -132,21 +117,7 @@ func (p *Parser) newModuleParser(moduleFS fs.FS, moduleSource, modulePath, modul return mp } -func (p *Parser) Metrics() Metrics { - total := p.metrics - for _, child := range p.children { - metrics := child.Metrics() - total.Counts.Files += metrics.Counts.Files - total.Counts.Blocks += metrics.Counts.Blocks - total.Timings.ParseDuration += metrics.Timings.ParseDuration - total.Timings.DiskIODuration += metrics.Timings.DiskIODuration - // NOTE: we don't add module count - this has already propagated to the top level - } - return total -} - func (p *Parser) ParseFile(_ context.Context, fullPath string) error { - diskStart := time.Now() isJSON := strings.HasSuffix(fullPath, ".tf.json") isHCL := strings.HasSuffix(fullPath, ".tf") @@ -165,14 +136,13 @@ func (p *Parser) ParseFile(_ context.Context, fullPath string) error { if err != nil { return err } - p.metrics.Timings.DiskIODuration += time.Since(diskStart) + if dir := path.Dir(fullPath); p.projectRoot == "" { p.debug.Log("Setting project/module root to '%s'", dir) p.projectRoot = dir p.modulePath = dir } - start := time.Now() var file *hcl.File var diag hcl.Diagnostics @@ -188,8 +158,7 @@ func (p *Parser) ParseFile(_ context.Context, fullPath string) error { file: file, path: fullPath, }) - p.metrics.Counts.Files++ - p.metrics.Timings.ParseDuration += time.Since(start) + p.debug.Log("Added file %s.", fullPath) return nil } @@ -270,8 +239,6 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, } p.debug.Log("Read %d block(s) and %d ignore(s) for module '%s' (%d file[s])...", len(blocks), len(ignores), p.moduleName, len(p.files)) - p.metrics.Counts.Blocks = len(blocks) - var inputVars map[string]cty.Value if p.moduleBlock != nil { inputVars = p.moduleBlock.Values().AsValueMap() @@ -312,9 +279,7 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, p.allowDownloads, p.skipCachedModules, ) - modules, fsMap, parseDuration := evaluator.EvaluateAll(ctx) - p.metrics.Counts.Modules = len(modules) - p.metrics.Timings.ParseDuration = parseDuration + modules, fsMap := evaluator.EvaluateAll(ctx) p.debug.Log("Finished parsing module '%s'.", p.moduleName) p.fsMap = fsMap return modules, evaluator.exportOutputs(), nil diff --git a/pkg/iac/scanners/terraform/performance_test.go b/pkg/iac/scanners/terraform/performance_test.go index f4a390a3b2cc..9015aa25b076 100644 --- a/pkg/iac/scanners/terraform/performance_test.go +++ b/pkg/iac/scanners/terraform/performance_test.go @@ -9,7 +9,7 @@ import ( "github.com/aquasecurity/trivy/internal/testutil" "github.com/aquasecurity/trivy/pkg/iac/rules" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/executor" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" + "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" ) func BenchmarkCalculate(b *testing.B) { @@ -21,7 +21,7 @@ func BenchmarkCalculate(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - p := parser2.New(f, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(f, "", parser.OptionStopOnHCLError(true)) if err := p.ParseFS(context.TODO(), "project"); err != nil { b.Fatal(err) } @@ -29,7 +29,7 @@ func BenchmarkCalculate(b *testing.B) { if err != nil { b.Fatal(err) } - _, _, _ = executor.New().Execute(modules) + executor.New().Execute(modules) } } diff --git a/pkg/iac/scanners/terraform/scanner.go b/pkg/iac/scanners/terraform/scanner.go index 5176b6471355..f5a3554d002d 100644 --- a/pkg/iac/scanners/terraform/scanner.go +++ b/pkg/iac/scanners/terraform/scanner.go @@ -10,7 +10,6 @@ import ( "sort" "strings" "sync" - "time" "github.com/aquasecurity/trivy/pkg/extrafs" "github.com/aquasecurity/trivy/pkg/iac/debug" @@ -21,7 +20,6 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/executor" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" - "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser/resolvers" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/aquasecurity/trivy/pkg/iac/types" ) @@ -120,14 +118,6 @@ func (s *Scanner) SetDataFilesystem(_ fs.FS) { } func (s *Scanner) SetRegoErrorLimit(_ int) {} -type Metrics struct { - Parser parser.Metrics - Executor executor.Metrics - Timings struct { - Total time.Duration - } -} - func New(opts ...options.ScannerOption) *Scanner { s := &Scanner{ dirs: make(map[string]struct{}), @@ -139,11 +129,6 @@ func New(opts ...options.ScannerOption) *Scanner { return s } -func (s *Scanner) ScanFS(ctx context.Context, target fs.FS, dir string) (scan.Results, error) { - results, _, err := s.ScanFSWithMetrics(ctx, target, dir) - return results, err -} - func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { s.Lock() defer s.Unlock() @@ -167,8 +152,7 @@ type terraformRootModule struct { fsMap map[string]fs.FS } -func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir string) (scan.Results, Metrics, error) { - var metrics Metrics +func (s *Scanner) ScanFS(ctx context.Context, target fs.FS, dir string) (scan.Results, error) { s.debug.Log("Scanning [%s] at '%s'...", target, dir) @@ -178,12 +162,12 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin if len(modulePaths) == 0 { s.debug.Log("no modules found") - return nil, metrics, nil + return nil, nil } regoScanner, err := s.initRegoScanner(target) if err != nil { - return nil, metrics, err + return nil, err } s.execLock.Lock() @@ -195,7 +179,7 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin p := parser.New(target, "", s.parserOpt...) rootDirs, err := p.FindRootModules(ctx, modulePaths) if err != nil { - return nil, metrics, fmt.Errorf("failed to find root modules: %w", err) + return nil, fmt.Errorf("failed to find root modules: %w", err) } rootModules := make([]terraformRootModule, 0, len(rootDirs)) @@ -208,21 +192,14 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin p := parser.New(target, "", s.parserOpt...) if err := p.ParseFS(ctx, dir); err != nil { - return nil, metrics, err + return nil, err } modules, _, err := p.EvaluateAll(ctx) if err != nil { - return nil, metrics, err + return nil, err } - parserMetrics := p.Metrics() - metrics.Parser.Counts.Blocks += parserMetrics.Counts.Blocks - metrics.Parser.Counts.Modules += parserMetrics.Counts.Modules - metrics.Parser.Counts.Files += parserMetrics.Counts.Files - metrics.Parser.Timings.DiskIODuration += parserMetrics.Timings.DiskIODuration - metrics.Parser.Timings.ParseDuration += parserMetrics.Timings.ParseDuration - rootModules = append(rootModules, terraformRootModule{ rootPath: dir, childs: modules, @@ -234,9 +211,9 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin s.execLock.RLock() e := executor.New(s.executorOpt...) s.execLock.RUnlock() - results, execMetrics, err := e.Execute(module.childs) + results, err := e.Execute(module.childs) if err != nil { - return nil, metrics, err + return nil, err } for i, result := range results { @@ -256,27 +233,10 @@ func (s *Scanner) ScanFSWithMetrics(ctx context.Context, target fs.FS, dir strin } } - metrics.Executor.Counts.Passed += execMetrics.Counts.Passed - metrics.Executor.Counts.Failed += execMetrics.Counts.Failed - metrics.Executor.Counts.Ignored += execMetrics.Counts.Ignored - metrics.Executor.Counts.Critical += execMetrics.Counts.Critical - metrics.Executor.Counts.High += execMetrics.Counts.High - metrics.Executor.Counts.Medium += execMetrics.Counts.Medium - metrics.Executor.Counts.Low += execMetrics.Counts.Low - metrics.Executor.Timings.Adaptation += execMetrics.Timings.Adaptation - metrics.Executor.Timings.RunningChecks += execMetrics.Timings.RunningChecks - allResults = append(allResults, results...) } - metrics.Parser.Counts.ModuleDownloads = resolvers.Remote.GetDownloadCount() - - metrics.Timings.Total += metrics.Parser.Timings.DiskIODuration - metrics.Timings.Total += metrics.Parser.Timings.ParseDuration - metrics.Timings.Total += metrics.Executor.Timings.Adaptation - metrics.Timings.Total += metrics.Executor.Timings.RunningChecks - - return allResults, metrics, nil + return allResults, nil } func (s *Scanner) removeNestedDirs(dirs []string) []string { diff --git a/pkg/iac/scanners/terraform/scanner_test.go b/pkg/iac/scanners/terraform/scanner_test.go index dbc2d67c3c64..020954d811db 100644 --- a/pkg/iac/scanners/terraform/scanner_test.go +++ b/pkg/iac/scanners/terraform/scanner_test.go @@ -63,7 +63,7 @@ func scanWithOptions(t *testing.T, code string, opt ...options.ScannerOption) sc }) scanner := New(opt...) - results, _, err := scanner.ScanFSWithMetrics(context.TODO(), fs, "project") + results, err := scanner.ScanFS(context.TODO(), fs, "project") require.NoError(t, err) return results } @@ -338,7 +338,7 @@ cause := bucket.name options.ScannerWithPolicyNamespaces(test.includedNamespaces...), ) - results, _, err := scanner.ScanFSWithMetrics(context.TODO(), fs, "code") + results, err := scanner.ScanFS(context.TODO(), fs, "code") require.NoError(t, err) var found bool @@ -376,7 +376,7 @@ resource "aws_s3_bucket" "my-bucket" { }), ) - _, _, err := scanner.ScanFSWithMetrics(context.TODO(), fs, "code") + _, err := scanner.ScanFS(context.TODO(), fs, "code") require.NoError(t, err) assert.Equal(t, 1, len(actual.AWS.S3.Buckets)) diff --git a/pkg/iac/scanners/terraform/setup_test.go b/pkg/iac/scanners/terraform/setup_test.go index c1f1aeb2e8ca..84bf3fdcc338 100644 --- a/pkg/iac/scanners/terraform/setup_test.go +++ b/pkg/iac/scanners/terraform/setup_test.go @@ -7,7 +7,7 @@ import ( "github.com/aquasecurity/trivy/internal/testutil" "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" - parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" + "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/stretchr/testify/require" ) @@ -17,7 +17,7 @@ func createModulesFromSource(t *testing.T, source string, ext string) terraform. "source" + ext: source, }) - p := parser2.New(fs, "", parser2.OptionStopOnHCLError(true)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) if err := p.ParseFS(context.TODO(), "."); err != nil { t.Fatal(err) } @@ -51,7 +51,7 @@ func scanJSON(t *testing.T, source string) scan.Results { }) s := New(options.ScannerWithEmbeddedPolicies(true), options.ScannerWithEmbeddedLibraries(true)) - results, _, err := s.ScanFSWithMetrics(context.TODO(), fs, ".") + results, err := s.ScanFS(context.TODO(), fs, ".") require.NoError(t, err) return results }