From bfdf5cfc305c6617e1030ea16b5632c9137c916e Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Fri, 23 Aug 2024 10:27:17 +0600 Subject: [PATCH] refactor(misconf): use slog (#7295) Signed-off-by: nikpivkin --- pkg/commands/artifact/run.go | 1 - pkg/iac/debug/debug.go | 38 ----- pkg/iac/rego/load.go | 39 +++-- pkg/iac/rego/load_test.go | 8 +- pkg/iac/rego/scanner.go | 34 +++-- pkg/iac/rego/scanner_test.go | 4 - pkg/iac/scanners/azure/arm/parser/parser.go | 46 +++--- .../scanners/azure/arm/parser/parser_test.go | 6 +- pkg/iac/scanners/azure/arm/scanner.go | 15 +- .../cloudformation/parser/file_context.go | 12 +- .../parser/file_context_test.go | 46 +++++- .../scanners/cloudformation/parser/parser.go | 55 ++++--- .../cloudformation/parser/resource.go | 2 +- pkg/iac/scanners/cloudformation/scanner.go | 27 ++-- .../cloudformation/test/cf_scanning_test.go | 17 --- pkg/iac/scanners/dockerfile/parser/parser.go | 21 +-- pkg/iac/scanners/dockerfile/scanner.go | 14 +- pkg/iac/scanners/dockerfile/scanner_test.go | 2 - pkg/iac/scanners/helm/options.go | 29 ++-- pkg/iac/scanners/helm/parser/option.go | 53 +++---- pkg/iac/scanners/helm/parser/parser.go | 37 +---- pkg/iac/scanners/helm/parser/parser_tar.go | 5 +- pkg/iac/scanners/helm/scanner.go | 22 ++- pkg/iac/scanners/helm/test/option_test.go | 9 +- pkg/iac/scanners/json/parser/parser.go | 22 +-- pkg/iac/scanners/json/scanner.go | 16 +-- pkg/iac/scanners/kubernetes/parser/parser.go | 21 +-- pkg/iac/scanners/kubernetes/scanner.go | 16 +-- pkg/iac/scanners/kubernetes/scanner_test.go | 2 - pkg/iac/scanners/options/parser.go | 16 --- pkg/iac/scanners/options/scanner.go | 8 -- .../scanners/terraform/executor/executor.go | 28 ++-- pkg/iac/scanners/terraform/executor/option.go | 9 -- pkg/iac/scanners/terraform/fs_test.go | 1 - pkg/iac/scanners/terraform/module_test.go | 13 +- pkg/iac/scanners/terraform/options.go | 2 +- .../scanners/terraform/parser/evaluator.go | 99 +++++++++---- .../scanners/terraform/parser/load_module.go | 24 +++- .../terraform/parser/module_retrieval.go | 6 +- pkg/iac/scanners/terraform/parser/option.go | 71 +++------- pkg/iac/scanners/terraform/parser/parser.go | 106 +++++++------- .../scanners/terraform/parser/parser_test.go | 41 +++++- .../terraform/parser/resolvers/cache.go | 10 +- .../terraform/parser/resolvers/local.go | 10 +- .../terraform/parser/resolvers/options.go | 8 +- .../terraform/parser/resolvers/registry.go | 18 ++- .../terraform/parser/resolvers/remote.go | 16 ++- pkg/iac/scanners/terraform/scanner.go | 44 +++--- .../terraform/scanner_integration_test.go | 16 --- pkg/iac/scanners/terraform/scanner_test.go | 134 +----------------- .../terraformplan/snapshot/scanner_test.go | 3 - .../terraformplan/tfjson/parser/option.go | 17 --- .../terraformplan/tfjson/parser/parser.go | 21 +-- .../scanners/terraformplan/tfjson/scanner.go | 20 +-- .../terraformplan/tfjson/scanner_test.go | 8 +- pkg/iac/scanners/toml/parser/parser.go | 22 +-- pkg/iac/scanners/toml/scanner.go | 16 +-- pkg/iac/scanners/yaml/parser/parser.go | 24 ++-- pkg/iac/scanners/yaml/scanner.go | 16 +-- pkg/log/logger.go | 16 --- pkg/misconf/scanner.go | 5 - 61 files changed, 566 insertions(+), 901 deletions(-) delete mode 100644 pkg/iac/debug/debug.go delete mode 100644 pkg/iac/scanners/options/parser.go delete mode 100644 pkg/iac/scanners/terraformplan/tfjson/parser/option.go diff --git a/pkg/commands/artifact/run.go b/pkg/commands/artifact/run.go index 59b3313d2f15..fa454c0cd276 100644 --- a/pkg/commands/artifact/run.go +++ b/pkg/commands/artifact/run.go @@ -640,7 +640,6 @@ func initMisconfScannerOption(opts flag.Options) (misconf.ScannerOption, error) } return misconf.ScannerOption{ - Debug: opts.Debug, Trace: opts.Trace, Namespaces: append(opts.CheckNamespaces, rego.BuiltinNamespaces()...), PolicyPaths: append(opts.CheckPaths, downloadedPolicyPaths...), diff --git a/pkg/iac/debug/debug.go b/pkg/iac/debug/debug.go deleted file mode 100644 index 489c777dd205..000000000000 --- a/pkg/iac/debug/debug.go +++ /dev/null @@ -1,38 +0,0 @@ -package debug - -import ( - "fmt" - "io" - "strings" - "time" -) - -const timeFormat = "04:05.000000000" - -type Logger struct { - writer io.Writer - prefix string -} - -func New(w io.Writer, parts ...string) Logger { - return Logger{ - writer: w, - prefix: strings.Join(parts, "."), - } -} - -func (l *Logger) Extend(parts ...string) Logger { - return Logger{ - writer: l.writer, - prefix: strings.Join(append([]string{l.prefix}, parts...), "."), - } -} - -func (l *Logger) Log(format string, args ...any) { - if l.writer == nil { - return - } - message := fmt.Sprintf(format, args...) - line := fmt.Sprintf("%s %-32s %s\n", time.Now().Format(timeFormat), l.prefix, message) - _, _ = l.writer.Write([]byte(line)) -} diff --git a/pkg/iac/rego/load.go b/pkg/iac/rego/load.go index bd474a23be4d..249e55992c2c 100644 --- a/pkg/iac/rego/load.go +++ b/pkg/iac/rego/load.go @@ -10,6 +10,8 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/samber/lo" + + "github.com/aquasecurity/trivy/pkg/log" ) var builtinNamespaces = map[string]struct{}{ @@ -61,14 +63,14 @@ func (s *Scanner) loadEmbedded() error { return fmt.Errorf("failed to load embedded rego libraries: %w", err) } s.embeddedLibs = loaded - s.debug.Log("Loaded %d embedded libraries.", len(loaded)) + s.logger.Debug("Embedded libraries are loaded", log.Int("count", len(loaded))) loaded, err = LoadEmbeddedPolicies() if err != nil { - return fmt.Errorf("failed to load embedded rego policies: %w", err) + return fmt.Errorf("failed to load embedded rego checks: %w", err) } s.embeddedChecks = loaded - s.debug.Log("Loaded %d embedded policies.", len(loaded)) + s.logger.Debug("Embedded checks are loaded", log.Int("count", len(loaded))) return nil } @@ -80,7 +82,7 @@ func (s *Scanner) LoadPolicies(enableEmbeddedLibraries, enableEmbeddedPolicies b } if s.policyFS != nil { - s.debug.Log("Overriding filesystem for checks!") + s.logger.Debug("Overriding filesystem for checks") srcFS = s.policyFS } @@ -105,7 +107,7 @@ func (s *Scanner) LoadPolicies(enableEmbeddedLibraries, enableEmbeddedPolicies b for name, policy := range loaded { s.policies[name] = policy } - s.debug.Log("Loaded %d checks from disk.", len(loaded)) + s.logger.Debug("Checks from disk are loaded", log.Int("count", len(loaded))) } if len(readers) > 0 { @@ -116,7 +118,7 @@ func (s *Scanner) LoadPolicies(enableEmbeddedLibraries, enableEmbeddedPolicies b for name, policy := range loaded { s.policies[name] = policy } - s.debug.Log("Loaded %d checks from reader(s).", len(loaded)) + s.logger.Debug("Checks from readers are loaded", log.Int("count", len(loaded))) } // gather namespaces @@ -132,7 +134,7 @@ func (s *Scanner) LoadPolicies(enableEmbeddedLibraries, enableEmbeddedPolicies b dataFS := srcFS if s.dataFS != nil { - s.debug.Log("Overriding filesystem for data!") + s.logger.Debug("Overriding filesystem for data") dataFS = s.dataFS } store, err := initStore(dataFS, s.dataDirs, namespaces) @@ -168,15 +170,19 @@ func (s *Scanner) fallbackChecks(compiler *ast.Compiler) { continue } - s.debug.Log("Error occurred while parsing: %s, %s. Trying to fallback to embedded check.", loc, e.Error()) + s.logger.Error( + "Error occurred while parsing. Trying to fallback to embedded check", + log.FilePath(loc), + log.Err(e), + ) embedded := s.findMatchedEmbeddedCheck(badPolicy) if embedded == nil { - s.debug.Log("Failed to find embedded check: %s", loc) + s.logger.Error("Failed to find embedded check, skipping", log.FilePath(loc)) continue } - s.debug.Log("Found embedded check: %s", embedded.Package.Location.File) + s.logger.Debug("Found embedded check", log.FilePath(embedded.Package.Location.File)) delete(s.policies, loc) // remove bad check s.policies[embedded.Package.Location.File] = embedded delete(s.embeddedChecks, embedded.Package.Location.File) // avoid infinite loop if embedded check contains ref error @@ -214,7 +220,7 @@ func (s *Scanner) findMatchedEmbeddedCheck(badPolicy *ast.Module) *ast.Module { func (s *Scanner) prunePoliciesWithError(compiler *ast.Compiler) error { if len(compiler.Errors) > s.regoErrorLimit { - s.debug.Log("Error(s) occurred while loading checks") + s.logger.Error("Error(s) occurred while loading checks") return compiler.Errors } @@ -222,7 +228,10 @@ func (s *Scanner) prunePoliciesWithError(compiler *ast.Compiler) error { if e.Location == nil { continue } - s.debug.Log("Error occurred while parsing: %s, %s", e.Location.File, e.Error()) + s.logger.Error( + "Error occurred while parsing", + log.FilePath(e.Location.File), log.Err(e), + ) delete(s.policies, e.Location.File) } return nil @@ -282,7 +291,11 @@ func (s *Scanner) filterModules(retriever *MetadataRetriever) error { return err } if len(meta.InputOptions.Selectors) == 0 { - s.debug.Log("WARNING: Module %s has no input selectors - it will be loaded for all inputs!", name) + s.logger.Warn( + "Module has no input selectors - it will be loaded for all inputs!", + log.FilePath(module.Package.Location.File), + log.String("module", name), + ) filtered[name] = module continue } diff --git a/pkg/iac/rego/load_test.go b/pkg/iac/rego/load_test.go index ede06e4610ed..e0c136562d45 100644 --- a/pkg/iac/rego/load_test.go +++ b/pkg/iac/rego/load_test.go @@ -5,6 +5,7 @@ import ( "embed" "fmt" "io" + "log/slog" "strings" "testing" "testing/fstest" @@ -17,6 +18,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) //go:embed all:testdata/policies @@ -28,10 +30,10 @@ var embeddedChecksFS embed.FS func Test_RegoScanning_WithSomeInvalidPolicies(t *testing.T) { t.Run("allow no errors", func(t *testing.T) { var debugBuf bytes.Buffer + slog.SetDefault(log.New(log.NewHandler(&debugBuf, nil))) scanner := rego.NewScanner( types.SourceDockerfile, options.ScannerWithRegoErrorLimits(0), - options.ScannerWithDebug(&debugBuf), ) err := scanner.LoadPolicies(false, false, testEmbedFS, []string{"."}, nil) @@ -41,16 +43,16 @@ func Test_RegoScanning_WithSomeInvalidPolicies(t *testing.T) { t.Run("allow up to max 1 error", func(t *testing.T) { var debugBuf bytes.Buffer + slog.SetDefault(log.New(log.NewHandler(&debugBuf, nil))) scanner := rego.NewScanner( types.SourceDockerfile, options.ScannerWithRegoErrorLimits(1), - options.ScannerWithDebug(&debugBuf), ) err := scanner.LoadPolicies(false, false, testEmbedFS, []string{"."}, nil) require.NoError(t, err) - assert.Contains(t, debugBuf.String(), "Error occurred while parsing: testdata/policies/invalid.rego, testdata/policies/invalid.rego:7") + assert.Contains(t, debugBuf.String(), "Error occurred while parsing\tfile_path=\"testdata/policies/invalid.rego\" err=\"testdata/policies/invalid.rego:7") }) t.Run("schema does not exist", func(t *testing.T) { diff --git a/pkg/iac/rego/scanner.go b/pkg/iac/rego/scanner.go index fe0553d6bc00..23a1e04bc2b8 100644 --- a/pkg/iac/rego/scanner.go +++ b/pkg/iac/rego/scanner.go @@ -15,13 +15,13 @@ import ( "github.com/open-policy-agent/opa/storage" "github.com/open-policy-agent/opa/util" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/providers" "github.com/aquasecurity/trivy/pkg/iac/rego/schemas" "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var checkTypesWithSubtype = map[types.Source]struct{}{ @@ -51,7 +51,7 @@ type Scanner struct { runtimeValues *ast.Term compiler *ast.Compiler regoErrorLimit int - debug debug.Logger + logger *log.Logger traceWriter io.Writer tracePerResult bool retriever *MetadataRetriever @@ -113,10 +113,6 @@ func (s *Scanner) SetPolicyReaders(_ []io.Reader) { // NOTE: Policy readers option not applicable for rego, policies are loaded on-demand by other scanners. } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "rego", "scanner") -} - func (s *Scanner) SetTraceWriter(writer io.Writer) { s.traceWriter = writer } @@ -166,6 +162,7 @@ func NewScanner(source types.Source, opts ...options.ScannerOption) *Scanner { sourceType: source, ruleNamespaces: make(map[string]struct{}), runtimeValues: addRuntimeValues(), + logger: log.WithPrefix("rego"), customSchemas: make(map[string][]byte), } @@ -183,10 +180,6 @@ func NewScanner(source types.Source, opts ...options.ScannerOption) *Scanner { return s } -func (s *Scanner) SetParentDebugLogger(l debug.Logger) { - s.debug = l.Extend("rego") -} - func (s *Scanner) runQuery(ctx context.Context, query string, input ast.Value, disableTracing bool) (rego.ResultSet, []string, error) { trace := (s.traceWriter != nil || s.tracePerResult) && !disableTracing @@ -247,7 +240,7 @@ func GetInputsContents(inputs []Input) []any { func (s *Scanner) ScanInput(ctx context.Context, inputs ...Input) (scan.Results, error) { - s.debug.Log("Scanning %d inputs...", len(inputs)) + s.logger.Debug("Scannning inputs", "count", len(inputs)) var results scan.Results @@ -267,9 +260,11 @@ func (s *Scanner) ScanInput(ctx context.Context, inputs ...Input) (scan.Results, staticMeta, err := s.retriever.RetrieveMetadata(ctx, module, GetInputsContents(inputs)...) if err != nil { - s.debug.Log( - "Error occurred while retrieving metadata from check %q: %s", - module.Package.Location.File, err) + s.logger.Error( + "Error occurred while retrieving metadata from check", + log.FilePath(module.Package.Location.File), + log.Err(err), + ) continue } @@ -300,9 +295,12 @@ func (s *Scanner) ScanInput(ctx context.Context, inputs ...Input) (scan.Results, if isEnforcedRule(ruleName) { ruleResults, err := s.applyRule(ctx, namespace, ruleName, inputs, staticMeta.InputOptions.Combined) if err != nil { - s.debug.Log( - "Error occurred while applying rule %q from check %q: %s", - ruleName, module.Package.Location.File, err) + s.logger.Error( + "Error occurred while applying rule from check", + log.String("rule", ruleName), + log.FilePath(module.Package.Location.File), + log.Err(err), + ) continue } results = append(results, s.embellishResultsWithRuleMetadata(ruleResults, *staticMeta)...) @@ -390,7 +388,7 @@ func (s *Scanner) applyRule(ctx context.Context, namespace, rule string, inputs s.trace("INPUT", input) parsedInput, err := parseRawInput(input.Contents) if err != nil { - s.debug.Log("Error occurred while parsing input: %s", err) + s.logger.Error("Error occurred while parsing input", log.Err(err)) continue } if ignored, err := s.isIgnored(ctx, namespace, rule, parsedInput); err != nil { diff --git a/pkg/iac/rego/scanner_test.go b/pkg/iac/rego/scanner_test.go index de2b7427ae8a..c453cc984b1c 100644 --- a/pkg/iac/rego/scanner_test.go +++ b/pkg/iac/rego/scanner_test.go @@ -998,10 +998,8 @@ deny { }, } - var buf bytes.Buffer scanner := NewScanner( types.SourceYAML, - options.ScannerWithDebug(&buf), ) require.NoError( t, @@ -1009,8 +1007,6 @@ deny { ) _, err := scanner.ScanInput(context.TODO(), Input{}) require.NoError(t, err) - assert.Contains(t, buf.String(), - `Error occurred while applying rule "deny" from check "checks/bad.rego"`) } func Test_RegoScanning_WithDeprecatedCheck(t *testing.T) { diff --git a/pkg/iac/scanners/azure/arm/parser/parser.go b/pkg/iac/scanners/azure/arm/parser/parser.go index 9fbf7b7019a8..53f9eb21431b 100644 --- a/pkg/iac/scanners/azure/arm/parser/parser.go +++ b/pkg/iac/scanners/azure/arm/parser/parser.go @@ -6,36 +6,28 @@ import ( "io" "io/fs" - "github.com/aquasecurity/trivy/pkg/iac/debug" - azure2 "github.com/aquasecurity/trivy/pkg/iac/scanners/azure" + "github.com/aquasecurity/trivy/pkg/iac/scanners/azure" "github.com/aquasecurity/trivy/pkg/iac/scanners/azure/arm/parser/armjson" "github.com/aquasecurity/trivy/pkg/iac/scanners/azure/resolver" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) type Parser struct { targetFS fs.FS - debug debug.Logger + logger *log.Logger } -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "azure", "arm") -} - -func New(targetFS fs.FS, opts ...options.ParserOption) *Parser { - p := &Parser{ +func New(targetFS fs.FS) *Parser { + return &Parser{ targetFS: targetFS, + logger: log.WithPrefix("arm parser"), } - for _, opt := range opts { - opt(p) - } - return p } -func (p *Parser) ParseFS(ctx context.Context, dir string) ([]azure2.Deployment, error) { +func (p *Parser) ParseFS(ctx context.Context, dir string) ([]azure.Deployment, error) { - var deployments []azure2.Deployment + var deployments []azure.Deployment if err := fs.WalkDir(p.targetFS, dir, func(path string, entry fs.DirEntry, err error) error { if err != nil { @@ -69,7 +61,7 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) ([]azure2.Deployment, return deployments, nil } -func (p *Parser) parseFile(r io.Reader, filename string) (*azure2.Deployment, error) { +func (p *Parser) parseFile(r io.Reader, filename string) (*azure.Deployment, error) { var template Template data, err := io.ReadAll(r) if err != nil { @@ -86,11 +78,11 @@ func (p *Parser) parseFile(r io.Reader, filename string) (*azure2.Deployment, er return p.convertTemplate(template), nil } -func (p *Parser) convertTemplate(template Template) *azure2.Deployment { +func (p *Parser) convertTemplate(template Template) *azure.Deployment { - deployment := azure2.Deployment{ + deployment := azure.Deployment{ Metadata: template.Metadata, - TargetScope: azure2.ScopeResourceGroup, // TODO: override from --resource-group? + TargetScope: azure.ScopeResourceGroup, // TODO: override from --resource-group? Parameters: nil, Variables: nil, Resources: nil, @@ -103,8 +95,8 @@ func (p *Parser) convertTemplate(template Template) *azure2.Deployment { // TODO: the references passed here should probably not be the name - maybe params.NAME.DefaultValue? for name, param := range template.Parameters { - deployment.Parameters = append(deployment.Parameters, azure2.Parameter{ - Variable: azure2.Variable{ + deployment.Parameters = append(deployment.Parameters, azure.Parameter{ + Variable: azure.Variable{ Name: name, Value: param.DefaultValue, }, @@ -114,14 +106,14 @@ func (p *Parser) convertTemplate(template Template) *azure2.Deployment { } for name, variable := range template.Variables { - deployment.Variables = append(deployment.Variables, azure2.Variable{ + deployment.Variables = append(deployment.Variables, azure.Variable{ Name: name, Value: variable, }) } for name, output := range template.Outputs { - deployment.Outputs = append(deployment.Outputs, azure2.Output{ + deployment.Outputs = append(deployment.Outputs, azure.Output{ Name: name, Value: output, }) @@ -134,15 +126,15 @@ func (p *Parser) convertTemplate(template Template) *azure2.Deployment { return &deployment } -func (p *Parser) convertResource(input Resource) azure2.Resource { +func (p *Parser) convertResource(input Resource) azure.Resource { - var children []azure2.Resource + var children []azure.Resource for _, child := range input.Resources { children = append(children, p.convertResource(child)) } - resource := azure2.Resource{ + resource := azure.Resource{ Metadata: input.Metadata, APIVersion: input.APIVersion, Type: input.Type, diff --git a/pkg/iac/scanners/azure/arm/parser/parser_test.go b/pkg/iac/scanners/azure/arm/parser/parser_test.go index b16213d9bebd..ff1444a60655 100644 --- a/pkg/iac/scanners/azure/arm/parser/parser_test.go +++ b/pkg/iac/scanners/azure/arm/parser/parser_test.go @@ -3,7 +3,6 @@ package parser import ( "context" "io/fs" - "os" "testing" "github.com/liamg/memoryfs" @@ -12,7 +11,6 @@ import ( azure2 "github.com/aquasecurity/trivy/pkg/iac/scanners/azure" "github.com/aquasecurity/trivy/pkg/iac/scanners/azure/resolver" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" ) @@ -205,7 +203,7 @@ func TestParser_Parse(t *testing.T) { require.NoError(t, targetFS.WriteFile(filename, []byte(tt.input), 0644)) - p := New(targetFS, options.ParserWithDebug(os.Stderr)) + p := New(targetFS) got, err := p.ParseFS(context.Background(), ".") require.NoError(t, err) @@ -293,7 +291,7 @@ func Test_NestedResourceParsing(t *testing.T) { require.NoError(t, targetFS.WriteFile("nested.json", []byte(input), 0644)) - p := New(targetFS, options.ParserWithDebug(os.Stderr)) + p := New(targetFS) got, err := p.ParseFS(context.Background(), ".") require.NoError(t, err) require.Len(t, got, 1) diff --git a/pkg/iac/scanners/azure/arm/scanner.go b/pkg/iac/scanners/azure/arm/scanner.go index fd585a5955c5..66bab2b4f806 100644 --- a/pkg/iac/scanners/azure/arm/scanner.go +++ b/pkg/iac/scanners/azure/arm/scanner.go @@ -8,7 +8,6 @@ import ( "sync" "github.com/aquasecurity/trivy/pkg/iac/adapters/arm" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/rules" @@ -19,6 +18,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/state" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -27,8 +27,7 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex scannerOptions []options.ScannerOption - parserOptions []options.ParserOption - debug debug.Logger + logger *log.Logger frameworks []framework.Framework regoOnly bool loadEmbeddedPolicies bool @@ -53,6 +52,7 @@ func (s *Scanner) SetRegoOnly(regoOnly bool) { func New(opts ...options.ScannerOption) *Scanner { scanner := &Scanner{ scannerOptions: opts, + logger: log.WithPrefix("azure-arm"), } for _, opt := range opts { opt(scanner) @@ -64,11 +64,6 @@ func (s *Scanner) Name() string { return "Azure ARM" } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "azure", "arm") - s.parserOptions = append(s.parserOptions, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetPolicyDirs(dirs ...string) { s.policyDirs = dirs } @@ -109,7 +104,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) error { return nil } regoScanner := rego.NewScanner(types.SourceCloud, s.scannerOptions...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return err } @@ -118,7 +112,7 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) error { } func (s *Scanner) ScanFS(ctx context.Context, fsys fs.FS, dir string) (scan.Results, error) { - p := parser.New(fsys, s.parserOptions...) + p := parser.New(fsys) deployments, err := p.ParseFS(ctx, dir) if err != nil { return nil, err @@ -160,7 +154,6 @@ func (s *Scanner) scanDeployment(ctx context.Context, deployment azure.Deploymen continue } ruleResults := rule.Evaluate(deploymentState) - s.debug.Log("Found %d results for %s", len(ruleResults), rule.GetRule().AVDID) if len(ruleResults) > 0 { results = append(results, ruleResults...) } diff --git a/pkg/iac/scanners/cloudformation/parser/file_context.go b/pkg/iac/scanners/cloudformation/parser/file_context.go index 78a267cabb04..949add1ca7c4 100644 --- a/pkg/iac/scanners/cloudformation/parser/file_context.go +++ b/pkg/iac/scanners/cloudformation/parser/file_context.go @@ -54,10 +54,20 @@ func (t *FileContext) Metadata() iacTypes.Metadata { return iacTypes.NewMetadata(rng, NewCFReference("Template", rng).String()) } -func (t *FileContext) OverrideParameters(params map[string]any) { +func (t *FileContext) overrideParameters(params map[string]any) { for key := range t.Parameters { if val, ok := params[key]; ok { t.Parameters[key].UpdateDefault(val) } } } + +func (t *FileContext) missingParameterValues() []string { + var missing []string + for key := range t.Parameters { + if t.Parameters[key].inner.Default == nil { + missing = append(missing, key) + } + } + return missing +} diff --git a/pkg/iac/scanners/cloudformation/parser/file_context_test.go b/pkg/iac/scanners/cloudformation/parser/file_context_test.go index bbf5db4ddc39..80bdfa72eb05 100644 --- a/pkg/iac/scanners/cloudformation/parser/file_context_test.go +++ b/pkg/iac/scanners/cloudformation/parser/file_context_test.go @@ -1,6 +1,7 @@ package parser import ( + "slices" "testing" "github.com/stretchr/testify/assert" @@ -54,8 +55,51 @@ func TestFileContext_OverrideParameters(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.ctx.OverrideParameters(tt.arg) + tt.ctx.overrideParameters(tt.arg) assert.Equal(t, tt.expected, tt.ctx.Parameters) }) } } + +func TestFileContext_MissingParameterValues(t *testing.T) { + + tests := []struct { + name string + ctx FileContext + expected []string + }{ + { + name: "happy", + ctx: FileContext{ + Parameters: map[string]*Parameter{ + "BucketName": { + inner: parameterInner{ + Type: "String", + Default: "test", + }, + }, + "QueueName": { + inner: parameterInner{ + Type: "String", + }, + }, + "KMSKey": { + inner: parameterInner{ + Type: "String", + Default: nil, + }, + }, + }, + }, + expected: []string{"KMSKey", "QueueName"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := tt.ctx.missingParameterValues() + slices.Sort(got) + assert.Equal(t, tt.expected, got) + }) + } +} diff --git a/pkg/iac/scanners/cloudformation/parser/parser.go b/pkg/iac/scanners/cloudformation/parser/parser.go index d281b585035e..40d0035f6cd2 100644 --- a/pkg/iac/scanners/cloudformation/parser/parser.go +++ b/pkg/iac/scanners/cloudformation/parser/parser.go @@ -13,51 +13,42 @@ import ( "github.com/liamg/jfather" "gopkg.in/yaml.v3" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/ignore" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) -var _ options.ConfigurableParser = (*Parser)(nil) - type Parser struct { - debug debug.Logger + logger *log.Logger parameterFiles []string parameters map[string]any overridedParameters Parameters configsFS fs.FS } -func WithParameters(params map[string]any) options.ParserOption { - return func(cp options.ConfigurableParser) { - if p, ok := cp.(*Parser); ok { - p.parameters = params - } - } -} +type Option func(*Parser) -func WithParameterFiles(files ...string) options.ParserOption { - return func(cp options.ConfigurableParser) { - if p, ok := cp.(*Parser); ok { - p.parameterFiles = files - } +func WithParameters(params map[string]any) Option { + return func(p *Parser) { + p.parameters = params } } -func WithConfigsFS(fsys fs.FS) options.ParserOption { - return func(cp options.ConfigurableParser) { - if p, ok := cp.(*Parser); ok { - p.configsFS = fsys - } +func WithParameterFiles(files ...string) Option { + return func(p *Parser) { + p.parameterFiles = files } } -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "cloudformation", "parser") +func WithConfigsFS(fsys fs.FS) Option { + return func(p *Parser) { + p.configsFS = fsys + } } -func New(opts ...options.ParserOption) *Parser { - p := &Parser{} +func New(opts ...Option) *Parser { + p := &Parser{ + logger: log.WithPrefix("cloudformation parser"), + } for _, option := range opts { option(p) } @@ -81,7 +72,7 @@ func (p *Parser) ParseFS(ctx context.Context, fsys fs.FS, dir string) (FileConte c, err := p.ParseFile(ctx, fsys, path) if err != nil { - p.debug.Log("Error parsing file '%s': %s", path, err) + p.logger.Error("Error parsing file", log.FilePath(path), log.Err(err)) return nil } contexts = append(contexts, c) @@ -149,13 +140,17 @@ func (p *Parser) ParseFile(ctx context.Context, fsys fs.FS, path string) (fctx * } } - fctx.OverrideParameters(p.overridedParameters) + fctx.overrideParameters(p.overridedParameters) + + if params := fctx.missingParameterValues(); len(params) > 0 { + p.logger.Warn("Missing parameter values", log.FilePath(path), log.String("parameters", strings.Join(params, ", "))) + } fctx.lines = lines fctx.SourceFormat = sourceFmt fctx.filepath = path - p.debug.Log("Context loaded from source %s", path) + p.logger.Debug("Context loaded from source", log.FilePath(path)) // the context must be set to conditions before resources for _, c := range fctx.Conditions { @@ -163,7 +158,7 @@ func (p *Parser) ParseFile(ctx context.Context, fsys fs.FS, path string) (fctx * } for name, r := range fctx.Resources { - r.ConfigureResource(name, fsys, path, fctx) + r.configureResource(name, fsys, path, fctx) } return fctx, nil diff --git a/pkg/iac/scanners/cloudformation/parser/resource.go b/pkg/iac/scanners/cloudformation/parser/resource.go index bd1351f234df..ed4b2834d573 100644 --- a/pkg/iac/scanners/cloudformation/parser/resource.go +++ b/pkg/iac/scanners/cloudformation/parser/resource.go @@ -23,7 +23,7 @@ type ResourceInner struct { Properties map[string]*Property `json:"Properties" yaml:"Properties"` } -func (r *Resource) ConfigureResource(id string, target fs.FS, filepath string, ctx *FileContext) { +func (r *Resource) configureResource(id string, target fs.FS, filepath string, ctx *FileContext) { r.setId(id) r.setFile(target, filepath) r.setContext(ctx) diff --git a/pkg/iac/scanners/cloudformation/scanner.go b/pkg/iac/scanners/cloudformation/scanner.go index cedd18ea6297..6db08c0d15bb 100644 --- a/pkg/iac/scanners/cloudformation/scanner.go +++ b/pkg/iac/scanners/cloudformation/scanner.go @@ -9,7 +9,6 @@ import ( "sync" adapter "github.com/aquasecurity/trivy/pkg/iac/adapters/cloudformation" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/rules" @@ -18,12 +17,13 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) func WithParameters(params map[string]any) options.ScannerOption { return func(cs options.ConfigurableScanner) { if s, ok := cs.(*Scanner); ok { - s.addParserOptions(parser.WithParameters(params)) + s.addParserOption(parser.WithParameters(params)) } } } @@ -31,7 +31,7 @@ func WithParameters(params map[string]any) options.ScannerOption { func WithParameterFiles(files ...string) options.ScannerOption { return func(cs options.ConfigurableScanner) { if s, ok := cs.(*Scanner); ok { - s.addParserOptions(parser.WithParameterFiles(files...)) + s.addParserOption(parser.WithParameterFiles(files...)) } } } @@ -39,7 +39,7 @@ func WithParameterFiles(files ...string) options.ScannerOption { func WithConfigsFS(fsys fs.FS) options.ScannerOption { return func(cs options.ConfigurableScanner) { if s, ok := cs.(*Scanner); ok { - s.addParserOptions(parser.WithConfigsFS(fsys)) + s.addParserOption(parser.WithConfigsFS(fsys)) } } } @@ -49,7 +49,7 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex - debug debug.Logger + logger *log.Logger policyDirs []string policyReaders []io.Reader parser *parser.Parser @@ -58,7 +58,7 @@ type Scanner struct { loadEmbeddedPolicies bool loadEmbeddedLibraries bool options []options.ScannerOption - parserOptions []options.ParserOption + parserOptions []parser.Option frameworks []framework.Framework spec string } @@ -66,7 +66,7 @@ type Scanner struct { func (s *Scanner) SetIncludeDeprecatedChecks(bool) {} func (s *Scanner) SetCustomSchemas(map[string][]byte) {} -func (s *Scanner) addParserOptions(opt options.ParserOption) { +func (s *Scanner) addParserOption(opt parser.Option) { s.parserOptions = append(s.parserOptions, opt) } @@ -98,11 +98,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "cloudformation", "scanner") - s.parserOptions = append(s.parserOptions, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetPolicyDirs(dirs ...string) { s.policyDirs = dirs } @@ -125,6 +120,7 @@ func (s *Scanner) SetPolicyNamespaces(_ ...string) {} func New(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("cloudformation scanner"), } for _, opt := range opts { opt(s) @@ -140,7 +136,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return s.regoScanner, nil } regoScanner := rego.NewScanner(types.SourceCloud, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } @@ -221,7 +216,6 @@ func (s *Scanner) scanFileContext(ctx context.Context, regoScanner *rego.Scanner } evalResult := rule.Evaluate(state) if len(evalResult) > 0 { - s.debug.Log("Found %d results for %s", len(evalResult), rule.GetRule().AVDID) for _, scanResult := range evalResult { ref := scanResult.Metadata().Reference() @@ -250,7 +244,10 @@ func (s *Scanner) scanFileContext(ctx context.Context, regoScanner *rego.Scanner results.Ignore(cfCtx.Ignores, nil) for _, ignored := range results.GetIgnored() { - s.debug.Log("Ignored '%s' at '%s'.", ignored.Rule().LongID(), ignored.Range()) + s.logger.Info("Ignore finding", + log.String("rule", ignored.Rule().LongID()), + log.String("range", ignored.Range().String()), + ) } return results, nil diff --git a/pkg/iac/scanners/cloudformation/test/cf_scanning_test.go b/pkg/iac/scanners/cloudformation/test/cf_scanning_test.go index 4ab3009d4dab..396963447ca9 100644 --- a/pkg/iac/scanners/cloudformation/test/cf_scanning_test.go +++ b/pkg/iac/scanners/cloudformation/test/cf_scanning_test.go @@ -1,7 +1,6 @@ package test import ( - "bytes" "context" "os" "testing" @@ -30,19 +29,3 @@ func Test_cloudformation_scanning_has_expected_errors(t *testing.T) { assert.NotEmpty(t, results.GetFailed()) } - -func Test_cloudformation_scanning_with_debug(t *testing.T) { - - debugWriter := bytes.NewBufferString("") - - scannerOptions := []options.ScannerOption{ - options.ScannerWithDebug(debugWriter), - } - cfScanner := cloudformation.New(scannerOptions...) - - _, err := cfScanner.ScanFS(context.TODO(), os.DirFS("./examples/bucket"), ".") - require.NoError(t, err) - - // check debug is as expected - assert.NotEmpty(t, debugWriter.String()) -} diff --git a/pkg/iac/scanners/dockerfile/parser/parser.go b/pkg/iac/scanners/dockerfile/parser/parser.go index 4255b4609b46..e92116ae3079 100644 --- a/pkg/iac/scanners/dockerfile/parser/parser.go +++ b/pkg/iac/scanners/dockerfile/parser/parser.go @@ -11,28 +11,19 @@ import ( "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/moby/buildkit/frontend/dockerfile/parser" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/providers/dockerfile" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) -var _ options.ConfigurableParser = (*Parser)(nil) - type Parser struct { - debug debug.Logger -} - -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "dockerfile", "parser") + logger *log.Logger } // New creates a new Dockerfile parser -func New(opts ...options.ParserOption) *Parser { - p := &Parser{} - for _, option := range opts { - option(p) +func New() *Parser { + return &Parser{ + logger: log.WithPrefix("dockerfile parser"), } - return p } func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[string]*dockerfile.Dockerfile, error) { @@ -53,7 +44,7 @@ func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[st df, err := p.ParseFile(ctx, target, path) if err != nil { - // TODO add debug for parse errors + p.logger.Error("Failed to parse Dockerfile", log.FilePath(path), log.Err(err)) return nil } files[path] = df diff --git a/pkg/iac/scanners/dockerfile/scanner.go b/pkg/iac/scanners/dockerfile/scanner.go index 6358aada12c2..e0d6ebd9ebde 100644 --- a/pkg/iac/scanners/dockerfile/scanner.go +++ b/pkg/iac/scanners/dockerfile/scanner.go @@ -6,7 +6,6 @@ import ( "io/fs" "sync" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" @@ -14,6 +13,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/dockerfile/parser" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -21,13 +21,12 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex - debug debug.Logger + logger *log.Logger policyDirs []string policyReaders []io.Reader parser *parser.Parser regoScanner *rego.Scanner options []options.ScannerOption - parserOpts []options.ParserOption frameworks []framework.Framework spec string loadEmbeddedLibraries bool @@ -64,11 +63,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "dockerfile", "scanner") - s.parserOpts = append(s.parserOpts, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetTraceWriter(_ io.Writer) { // handled by rego later - nothing to do for now... } @@ -104,6 +98,7 @@ func (s *Scanner) SetRegoErrorLimit(_ int) { func NewScanner(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("dockerfile scanner"), } for _, opt := range opts { opt(s) @@ -144,7 +139,7 @@ func (s *Scanner) ScanFile(ctx context.Context, fsys fs.FS, path string) (scan.R if err != nil { return nil, err } - s.debug.Log("Scanning %s...", path) + s.logger.Debug("Scanning", log.FilePath(path)) return s.scanRego(ctx, fsys, rego.Input{ Path: path, Contents: dockerfile.ToRego(), @@ -159,7 +154,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { } regoScanner := rego.NewScanner(types.SourceDockerfile, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } diff --git a/pkg/iac/scanners/dockerfile/scanner_test.go b/pkg/iac/scanners/dockerfile/scanner_test.go index 72c48f6a9f5e..437c37ec9660 100644 --- a/pkg/iac/scanners/dockerfile/scanner_test.go +++ b/pkg/iac/scanners/dockerfile/scanner_test.go @@ -566,12 +566,10 @@ COPY --from=dep /binary /` fs := testutil.CreateFS(t, regoMap) var traceBuf bytes.Buffer - var debugBuf bytes.Buffer scanner := NewScanner( options.ScannerWithPolicyDirs("rules"), options.ScannerWithTrace(&traceBuf), - options.ScannerWithDebug(&debugBuf), options.ScannerWithRegoErrorLimits(0), ) diff --git a/pkg/iac/scanners/helm/options.go b/pkg/iac/scanners/helm/options.go index 6ac412bf1e34..c4439d928845 100644 --- a/pkg/iac/scanners/helm/options.go +++ b/pkg/iac/scanners/helm/options.go @@ -5,55 +5,50 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/options" ) -type ConfigurableHelmScanner interface { - options.ConfigurableScanner - AddParserOptions(options ...options.ParserOption) -} - func ScannerWithValuesFile(paths ...string) options.ScannerOption { return func(s options.ConfigurableScanner) { - if helmScanner, ok := s.(ConfigurableHelmScanner); ok { - helmScanner.AddParserOptions(parser.OptionWithValuesFile(paths...)) + if helmScanner, ok := s.(*Scanner); ok { + helmScanner.addParserOptions(parser.OptionWithValuesFile(paths...)) } } } func ScannerWithValues(values ...string) options.ScannerOption { return func(s options.ConfigurableScanner) { - if helmScanner, ok := s.(ConfigurableHelmScanner); ok { - helmScanner.AddParserOptions(parser.OptionWithValues(values...)) + if helmScanner, ok := s.(*Scanner); ok { + helmScanner.addParserOptions(parser.OptionWithValues(values...)) } } } func ScannerWithFileValues(values ...string) options.ScannerOption { return func(s options.ConfigurableScanner) { - if helmScanner, ok := s.(ConfigurableHelmScanner); ok { - helmScanner.AddParserOptions(parser.OptionWithFileValues(values...)) + if helmScanner, ok := s.(*Scanner); ok { + helmScanner.addParserOptions(parser.OptionWithFileValues(values...)) } } } func ScannerWithStringValues(values ...string) options.ScannerOption { return func(s options.ConfigurableScanner) { - if helmScanner, ok := s.(ConfigurableHelmScanner); ok { - helmScanner.AddParserOptions(parser.OptionWithStringValues(values...)) + if helmScanner, ok := s.(*Scanner); ok { + helmScanner.addParserOptions(parser.OptionWithStringValues(values...)) } } } func ScannerWithAPIVersions(values ...string) options.ScannerOption { return func(s options.ConfigurableScanner) { - if helmScanner, ok := s.(ConfigurableHelmScanner); ok { - helmScanner.AddParserOptions(parser.OptionWithAPIVersions(values...)) + if helmScanner, ok := s.(*Scanner); ok { + helmScanner.addParserOptions(parser.OptionWithAPIVersions(values...)) } } } func ScannerWithKubeVersion(values string) options.ScannerOption { return func(s options.ConfigurableScanner) { - if helmScanner, ok := s.(ConfigurableHelmScanner); ok { - helmScanner.AddParserOptions(parser.OptionWithKubeVersion(values)) + if helmScanner, ok := s.(*Scanner); ok { + helmScanner.addParserOptions(parser.OptionWithKubeVersion(values)) } } } diff --git a/pkg/iac/scanners/helm/parser/option.go b/pkg/iac/scanners/helm/parser/option.go index 6de98d765182..0f2eae3cc291 100644 --- a/pkg/iac/scanners/helm/parser/option.go +++ b/pkg/iac/scanners/helm/parser/option.go @@ -1,9 +1,6 @@ package parser -import "github.com/aquasecurity/trivy/pkg/iac/scanners/options" - type ConfigurableHelmParser interface { - options.ConfigurableParser SetValuesFile(...string) SetValues(...string) SetFileValues(...string) @@ -12,50 +9,40 @@ type ConfigurableHelmParser interface { SetKubeVersion(string) } -func OptionWithValuesFile(paths ...string) options.ParserOption { - return func(p options.ConfigurableParser) { - if helmParser, ok := p.(ConfigurableHelmParser); ok { - helmParser.SetValuesFile(paths...) - } +type Option func(p *Parser) + +func OptionWithValuesFile(paths ...string) Option { + return func(p *Parser) { + p.valuesFiles = paths } } -func OptionWithValues(values ...string) options.ParserOption { - return func(p options.ConfigurableParser) { - if helmParser, ok := p.(ConfigurableHelmParser); ok { - helmParser.SetValues(values...) - } +func OptionWithValues(values ...string) Option { + return func(p *Parser) { + p.values = values } } -func OptionWithFileValues(values ...string) options.ParserOption { - return func(p options.ConfigurableParser) { - if helmParser, ok := p.(ConfigurableHelmParser); ok { - helmParser.SetValues(values...) - } +func OptionWithFileValues(values ...string) Option { + return func(p *Parser) { + p.fileValues = values } } -func OptionWithStringValues(values ...string) options.ParserOption { - return func(p options.ConfigurableParser) { - if helmParser, ok := p.(ConfigurableHelmParser); ok { - helmParser.SetValues(values...) - } +func OptionWithStringValues(values ...string) Option { + return func(p *Parser) { + p.stringValues = values } } -func OptionWithAPIVersions(values ...string) options.ParserOption { - return func(p options.ConfigurableParser) { - if helmParser, ok := p.(ConfigurableHelmParser); ok { - helmParser.SetAPIVersions(values...) - } +func OptionWithAPIVersions(values ...string) Option { + return func(p *Parser) { + p.apiVersions = values } } -func OptionWithKubeVersion(value string) options.ParserOption { - return func(p options.ConfigurableParser) { - if helmParser, ok := p.(ConfigurableHelmParser); ok { - helmParser.SetKubeVersion(value) - } +func OptionWithKubeVersion(value string) Option { + return func(p *Parser) { + p.kubeVersion = value } } diff --git a/pkg/iac/scanners/helm/parser/parser.go b/pkg/iac/scanners/helm/parser/parser.go index bf4a9fc91721..e38f032b6221 100644 --- a/pkg/iac/scanners/helm/parser/parser.go +++ b/pkg/iac/scanners/helm/parser/parser.go @@ -5,7 +5,6 @@ import ( "context" "errors" "fmt" - "io" "io/fs" "path/filepath" "regexp" @@ -21,19 +20,18 @@ import ( "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/releaseutil" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/detection" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) var manifestNameRegex = regexp.MustCompile("# Source: [^/]+/(.+)") type Parser struct { + logger *log.Logger helmClient *action.Install rootPath string ChartSource string filepaths []string - debug debug.Logger workingFS fs.FS valuesFiles []string values []string @@ -48,35 +46,7 @@ type ChartFile struct { ManifestContent string } -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "helm", "parser") -} - -func (p *Parser) SetValuesFile(s ...string) { - p.valuesFiles = s -} - -func (p *Parser) SetValues(values ...string) { - p.values = values -} - -func (p *Parser) SetFileValues(values ...string) { - p.fileValues = values -} - -func (p *Parser) SetStringValues(values ...string) { - p.stringValues = values -} - -func (p *Parser) SetAPIVersions(values ...string) { - p.apiVersions = values -} - -func (p *Parser) SetKubeVersion(value string) { - p.kubeVersion = value -} - -func New(path string, opts ...options.ParserOption) (*Parser, error) { +func New(path string, opts ...Option) (*Parser, error) { client := action.NewInstall(&action.Configuration{}) client.DryRun = true // don't do anything @@ -86,6 +56,7 @@ func New(path string, opts ...options.ParserOption) (*Parser, error) { p := &Parser{ helmClient: client, ChartSource: path, + logger: log.WithPrefix("helm parser"), } for _, option := range opts { diff --git a/pkg/iac/scanners/helm/parser/parser_tar.go b/pkg/iac/scanners/helm/parser/parser_tar.go index f8d692eaa977..8e2ba97a81d2 100644 --- a/pkg/iac/scanners/helm/parser/parser_tar.go +++ b/pkg/iac/scanners/helm/parser/parser_tar.go @@ -14,6 +14,7 @@ import ( "github.com/liamg/memoryfs" "github.com/aquasecurity/trivy/pkg/iac/detection" + "github.com/aquasecurity/trivy/pkg/log" ) var errSkipFS = errors.New("skip parse FS") @@ -75,13 +76,13 @@ func (p *Parser) addTarToFS(archivePath string) (fs.FS, error) { return nil, err } case tar.TypeReg: - p.debug.Log("Unpacking tar entry %s", targetPath) + p.logger.Debug("Unpacking tar entry", log.FilePath(targetPath)) if err := copyFile(tarFS, tr, targetPath); err != nil { return nil, err } case tar.TypeSymlink: if path.IsAbs(link) { - p.debug.Log("Symlink %s is absolute, skipping", link) + p.logger.Debug("Symlink is absolute, skipping", log.String("link", link)) continue } diff --git a/pkg/iac/scanners/helm/scanner.go b/pkg/iac/scanners/helm/scanner.go index 944187df4dee..305a116b56b7 100644 --- a/pkg/iac/scanners/helm/scanner.go +++ b/pkg/iac/scanners/helm/scanner.go @@ -11,7 +11,6 @@ import ( "github.com/liamg/memoryfs" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/detection" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" @@ -21,6 +20,7 @@ import ( kparser "github.com/aquasecurity/trivy/pkg/iac/scanners/kubernetes/parser" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -29,9 +29,9 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { policyDirs []string dataDirs []string - debug debug.Logger + logger *log.Logger options []options.ScannerOption - parserOptions []options.ParserOption + parserOptions []parser.Option policyReaders []io.Reader loadEmbeddedLibraries bool loadEmbeddedPolicies bool @@ -60,6 +60,7 @@ func (s *Scanner) SetFrameworks(frameworks []framework.Framework) { func New(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("helm scanner"), } for _, option := range opts { @@ -68,7 +69,7 @@ func New(opts ...options.ScannerOption) *Scanner { return s } -func (s *Scanner) AddParserOptions(opts ...options.ParserOption) { +func (s *Scanner) addParserOptions(opts ...parser.Option) { s.parserOptions = append(s.parserOptions, opts...) } @@ -88,11 +89,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "helm", "scanner") - s.parserOptions = append(s.parserOptions, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetTraceWriter(_ io.Writer) { // handled by rego later - nothing to do for now... } @@ -180,13 +176,16 @@ func (s *Scanner) getScanResults(path string, ctx context.Context, target fs.FS) chartFiles, err := helmParser.RenderedChartFiles() if err != nil { // not valid helm, maybe some other yaml etc., abort - s.debug.Log("Failed to render Chart files: %s", err) + s.logger.Error( + "Failed to render Chart files", + log.FilePath(path), log.Err(err), + ) return nil, nil } for _, file := range chartFiles { file := file - s.debug.Log("Processing rendered chart file: %s", file.TemplateFilePath) + s.logger.Debug("Processing rendered chart file", log.FilePath(file.TemplateFilePath)) manifests, err := kparser.New().Parse(strings.NewReader(file.ManifestContent), file.TemplateFilePath) if err != nil { @@ -229,7 +228,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) error { return nil } regoScanner := rego.NewScanner(types.SourceKubernetes, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return err } diff --git a/pkg/iac/scanners/helm/test/option_test.go b/pkg/iac/scanners/helm/test/option_test.go index 8efb03f16116..05bf96ee01d3 100644 --- a/pkg/iac/scanners/helm/test/option_test.go +++ b/pkg/iac/scanners/helm/test/option_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "github.com/aquasecurity/trivy/pkg/iac/scanners/helm/parser" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" ) func Test_helm_parser_with_options_with_values_file(t *testing.T) { @@ -34,7 +33,7 @@ func Test_helm_parser_with_options_with_values_file(t *testing.T) { t.Logf("Running test: %s", test.testName) - var opts []options.ParserOption + var opts []parser.Option if test.valuesFile != "" { opts = append(opts, parser.OptionWithValuesFile(test.valuesFile)) @@ -84,7 +83,7 @@ func Test_helm_parser_with_options_with_set_value(t *testing.T) { t.Logf("Running test: %s", test.testName) - var opts []options.ParserOption + var opts []parser.Option if test.valuesFile != "" { opts = append(opts, parser.OptionWithValuesFile(test.valuesFile)) @@ -138,7 +137,7 @@ func Test_helm_parser_with_options_with_api_versions(t *testing.T) { t.Logf("Running test: %s", test.testName) - var opts []options.ParserOption + var opts []parser.Option if len(test.apiVersions) > 0 { opts = append(opts, parser.OptionWithAPIVersions(test.apiVersions...)) @@ -195,7 +194,7 @@ func Test_helm_parser_with_options_with_kube_versions(t *testing.T) { t.Logf("Running test: %s", test.testName) - var opts []options.ParserOption + var opts []parser.Option opts = append(opts, parser.OptionWithKubeVersion(test.kubeVersion)) diff --git a/pkg/iac/scanners/json/parser/parser.go b/pkg/iac/scanners/json/parser/parser.go index 8f35794229ef..c97e54b40bd9 100644 --- a/pkg/iac/scanners/json/parser/parser.go +++ b/pkg/iac/scanners/json/parser/parser.go @@ -3,31 +3,21 @@ package parser import ( "context" "encoding/json" - "io" "io/fs" "path/filepath" - "github.com/aquasecurity/trivy/pkg/iac/debug" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) -var _ options.ConfigurableParser = (*Parser)(nil) - type Parser struct { - debug debug.Logger -} - -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "json", "parser") + logger *log.Logger } // New creates a new parser -func New(opts ...options.ParserOption) *Parser { - p := &Parser{} - for _, opt := range opts { - opt(p) +func New() *Parser { + return &Parser{ + logger: log.WithPrefix("json parser"), } - return p } func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[string]any, error) { @@ -48,7 +38,7 @@ func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[st df, err := p.ParseFile(ctx, target, path) if err != nil { - p.debug.Log("Parse error in '%s': %s", path, err) + p.logger.Error("Parse error", log.FilePath(path), log.Err(err)) return nil } diff --git a/pkg/iac/scanners/json/scanner.go b/pkg/iac/scanners/json/scanner.go index 7a18df363823..8991120be26f 100644 --- a/pkg/iac/scanners/json/scanner.go +++ b/pkg/iac/scanners/json/scanner.go @@ -6,7 +6,6 @@ import ( "io/fs" "sync" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" @@ -14,6 +13,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/json/parser" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -21,13 +21,12 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex - debug debug.Logger + logger *log.Logger policyDirs []string policyReaders []io.Reader parser *parser.Parser regoScanner *rego.Scanner options []options.ScannerOption - parserOpts []options.ParserOption frameworks []framework.Framework spec string loadEmbeddedPolicies bool @@ -60,11 +59,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "json", "scanner") - s.parserOpts = append(s.parserOpts, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetTraceWriter(_ io.Writer) { } @@ -90,11 +84,12 @@ func (s *Scanner) SetRegoErrorLimit(_ int) {} func NewScanner(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("json scanner"), + parser: parser.New(), } for _, opt := range opts { opt(s) } - s.parser = parser.New(s.parserOpts...) return s } @@ -134,7 +129,7 @@ func (s *Scanner) ScanFile(ctx context.Context, fsys fs.FS, path string) (scan.R if err != nil { return nil, err } - s.debug.Log("Scanning %s...", path) + s.logger.Debug("Scanning", log.FilePath(path)) return s.scanRego(ctx, fsys, rego.Input{ Path: path, Contents: parsed, @@ -148,7 +143,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return s.regoScanner, nil } regoScanner := rego.NewScanner(types.SourceJSON, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } diff --git a/pkg/iac/scanners/kubernetes/parser/parser.go b/pkg/iac/scanners/kubernetes/parser/parser.go index e2f225a9fb86..f3ca7d613562 100644 --- a/pkg/iac/scanners/kubernetes/parser/parser.go +++ b/pkg/iac/scanners/kubernetes/parser/parser.go @@ -13,27 +13,18 @@ import ( "gopkg.in/yaml.v3" kyaml "sigs.k8s.io/yaml" - "github.com/aquasecurity/trivy/pkg/iac/debug" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) -var _ options.ConfigurableParser = (*Parser)(nil) - type Parser struct { - debug debug.Logger -} - -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "kubernetes", "parser") + logger *log.Logger } // New creates a new K8s parser -func New(opts ...options.ParserOption) *Parser { - p := &Parser{} - for _, option := range opts { - option(p) +func New() *Parser { + return &Parser{ + logger: log.WithPrefix("k8s parser"), } - return p } func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[string][]any, error) { @@ -53,7 +44,7 @@ func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[st parsed, err := p.ParseFile(ctx, target, path) if err != nil { - p.debug.Log("Parse error in '%s': %s", path, err) + p.logger.Error("Parse error", log.FilePath(path), log.Err(err)) return nil } diff --git a/pkg/iac/scanners/kubernetes/scanner.go b/pkg/iac/scanners/kubernetes/scanner.go index 9612fe03ebe4..e9e9eacd73e8 100644 --- a/pkg/iac/scanners/kubernetes/scanner.go +++ b/pkg/iac/scanners/kubernetes/scanner.go @@ -10,7 +10,6 @@ import ( "github.com/liamg/memoryfs" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" @@ -18,6 +17,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/kubernetes/parser" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -25,9 +25,8 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex - debug debug.Logger + logger *log.Logger options []options.ScannerOption - parserOpts []options.ParserOption policyDirs []string policyReaders []io.Reader regoScanner *rego.Scanner @@ -63,11 +62,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "kubernetes", "scanner") - s.parserOpts = append(s.parserOpts, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetTraceWriter(_ io.Writer) { } @@ -94,11 +88,12 @@ func (s *Scanner) SetRegoErrorLimit(_ int) {} func NewScanner(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("k8s scanner"), + parser: parser.New(), } for _, opt := range opts { opt(s) } - s.parser = parser.New(s.parserOpts...) return s } @@ -113,7 +108,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return s.regoScanner, nil } regoScanner := rego.NewScanner(types.SourceKubernetes, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } @@ -163,7 +157,7 @@ func (s *Scanner) ScanFS(ctx context.Context, target fs.FS, dir string) (scan.Re return nil, err } - s.debug.Log("Scanning %d files...", len(inputs)) + s.logger.Debug("Scanning files", log.Int("count", len(inputs))) results, err := regoScanner.ScanInput(ctx, inputs...) if err != nil { return nil, err diff --git a/pkg/iac/scanners/kubernetes/scanner_test.go b/pkg/iac/scanners/kubernetes/scanner_test.go index a75173f67c0e..d972e52e8e80 100644 --- a/pkg/iac/scanners/kubernetes/scanner_test.go +++ b/pkg/iac/scanners/kubernetes/scanner_test.go @@ -486,7 +486,6 @@ deny[msg] { func Test_FileScanWithMetadata(t *testing.T) { results, err := NewScanner( - options.ScannerWithDebug(os.Stdout), options.ScannerWithTrace(os.Stdout), options.ScannerWithPolicyReader(strings.NewReader(`package defsec @@ -526,7 +525,6 @@ spec: func Test_FileScanExampleWithResultFunction(t *testing.T) { results, err := NewScanner( - options.ScannerWithDebug(os.Stdout), options.ScannerWithEmbeddedPolicies(true), options.ScannerWithEmbeddedLibraries(true), options.ScannerWithPolicyReader(strings.NewReader(`package defsec diff --git a/pkg/iac/scanners/options/parser.go b/pkg/iac/scanners/options/parser.go deleted file mode 100644 index e411126a6329..000000000000 --- a/pkg/iac/scanners/options/parser.go +++ /dev/null @@ -1,16 +0,0 @@ -package options - -import "io" - -type ConfigurableParser interface { - SetDebugWriter(io.Writer) -} - -type ParserOption func(s ConfigurableParser) - -// ParserWithDebug specifies an io.Writer for debug logs - if not set, they are discarded -func ParserWithDebug(w io.Writer) ParserOption { - return func(s ConfigurableParser) { - s.SetDebugWriter(w) - } -} diff --git a/pkg/iac/scanners/options/scanner.go b/pkg/iac/scanners/options/scanner.go index f5b3b982ee67..4f07c4d14b0a 100644 --- a/pkg/iac/scanners/options/scanner.go +++ b/pkg/iac/scanners/options/scanner.go @@ -8,7 +8,6 @@ import ( ) type ConfigurableScanner interface { - SetDebugWriter(io.Writer) SetTraceWriter(io.Writer) SetPerResultTracingEnabled(bool) SetPolicyDirs(...string) @@ -47,13 +46,6 @@ func ScannerWithPolicyReader(readers ...io.Reader) ScannerOption { } } -// ScannerWithDebug specifies an io.Writer for debug logs - if not set, they are discarded -func ScannerWithDebug(w io.Writer) ScannerOption { - return func(s ConfigurableScanner) { - s.SetDebugWriter(w) - } -} - func ScannerWithEmbeddedPolicies(embedded bool) ScannerOption { return func(s ConfigurableScanner) { s.SetUseEmbeddedPolicies(embedded) diff --git a/pkg/iac/scanners/terraform/executor/executor.go b/pkg/iac/scanners/terraform/executor/executor.go index 88dc1fa9801c..c6337a2bfd1d 100644 --- a/pkg/iac/scanners/terraform/executor/executor.go +++ b/pkg/iac/scanners/terraform/executor/executor.go @@ -8,7 +8,6 @@ import ( "github.com/zclconf/go-cty/cty" adapter "github.com/aquasecurity/trivy/pkg/iac/adapters/terraform" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/ignore" "github.com/aquasecurity/trivy/pkg/iac/rego" @@ -16,12 +15,13 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) // Executor scans HCL blocks by running all registered rules against them type Executor struct { workspaceName string - debug debug.Logger + logger *log.Logger resultsFilters []func(scan.Results) scan.Results regoScanner *rego.Scanner regoOnly bool @@ -32,6 +32,7 @@ type Executor struct { func New(options ...Option) *Executor { s := &Executor{ regoOnly: false, + logger: log.WithPrefix("terraform executor"), } for _, option := range options { option(s) @@ -41,31 +42,30 @@ func New(options ...Option) *Executor { func (e *Executor) Execute(modules terraform.Modules) (scan.Results, error) { - e.debug.Log("Adapting modules...") + e.logger.Debug("Adapting modules...") infra := adapter.Adapt(modules) - e.debug.Log("Adapted %d module(s) into defsec state data.", len(modules)) + e.logger.Debug("Adapted module(s) into state data.", log.Int("count", len(modules))) threads := runtime.NumCPU() if threads > 1 { threads-- } - e.debug.Log("Using max routines of %d", threads) + e.logger.Debug("Using max routines", log.Int("count", threads)) registeredRules := rules.GetRegistered(e.frameworks...) - e.debug.Log("Initialized %d rule(s).", len(registeredRules)) + e.logger.Debug("Initialized rule(s).", log.Int("count", len(registeredRules))) pool := NewPool(threads, registeredRules, modules, infra, e.regoScanner, e.regoOnly) - e.debug.Log("Created pool with %d worker(s) to apply rules.", threads) results, err := pool.Run() if err != nil { return nil, err } - e.debug.Log("Finished applying rules.") + e.logger.Debug("Finished applying rules.") - e.debug.Log("Applying ignores...") + e.logger.Debug("Applying ignores...") var ignores ignore.Rules for _, module := range modules { ignores = append(ignores, module.Ignores()...) @@ -79,7 +79,10 @@ func (e *Executor) Execute(modules terraform.Modules) (scan.Results, error) { results.Ignore(ignores, ignorers) for _, ignored := range results.GetIgnored() { - e.debug.Log("Ignored '%s' at '%s'.", ignored.Rule().LongID(), ignored.Range()) + e.logger.Info("Ignore finding", + log.String("rule", ignored.Rule().LongID()), + log.String("range", ignored.Range().String()), + ) } results = e.filterResults(results) @@ -91,11 +94,12 @@ func (e *Executor) Execute(modules terraform.Modules) (scan.Results, error) { func (e *Executor) filterResults(results scan.Results) scan.Results { if len(e.resultsFilters) > 0 && len(results) > 0 { before := len(results.GetIgnored()) - e.debug.Log("Applying %d results filters to %d results...", len(results), before) + e.logger.Debug("Applying results filters...") for _, filter := range e.resultsFilters { results = filter(results) } - e.debug.Log("Filtered out %d results.", len(results.GetIgnored())-before) + e.logger.Debug("Applied results filters.", + log.Int("count", len(results.GetIgnored())-before)) } return results diff --git a/pkg/iac/scanners/terraform/executor/option.go b/pkg/iac/scanners/terraform/executor/option.go index a58d72867b54..d2414878c98f 100644 --- a/pkg/iac/scanners/terraform/executor/option.go +++ b/pkg/iac/scanners/terraform/executor/option.go @@ -1,9 +1,6 @@ package executor import ( - "io" - - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" @@ -23,12 +20,6 @@ func OptionWithResultsFilter(f func(scan.Results) scan.Results) Option { } } -func OptionWithDebugWriter(w io.Writer) Option { - return func(s *Executor) { - s.debug = debug.New(w, "terraform", "executor") - } -} - func OptionWithWorkspaceName(workspaceName string) Option { return func(s *Executor) { s.workspaceName = workspaceName diff --git a/pkg/iac/scanners/terraform/fs_test.go b/pkg/iac/scanners/terraform/fs_test.go index 79b0844fe983..649062792514 100644 --- a/pkg/iac/scanners/terraform/fs_test.go +++ b/pkg/iac/scanners/terraform/fs_test.go @@ -13,7 +13,6 @@ import ( func Test_OS_FS(t *testing.T) { s := New( - options.ScannerWithDebug(os.Stderr), options.ScannerWithEmbeddedPolicies(true), options.ScannerWithEmbeddedLibraries(true), ) diff --git a/pkg/iac/scanners/terraform/module_test.go b/pkg/iac/scanners/terraform/module_test.go index 5469204e741f..d0d289a9d562 100644 --- a/pkg/iac/scanners/terraform/module_test.go +++ b/pkg/iac/scanners/terraform/module_test.go @@ -1,10 +1,7 @@ package terraform import ( - "bytes" "context" - "fmt" - "os" "testing" "github.com/stretchr/testify/require" @@ -14,7 +11,6 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/providers" "github.com/aquasecurity/trivy/pkg/iac/rules" "github.com/aquasecurity/trivy/pkg/iac/scan" - "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/severity" @@ -86,9 +82,7 @@ resource "problem" "uhoh" { `, }) - debug := bytes.NewBuffer([]byte{}) - - p := parser.New(fs, "", parser.OptionStopOnHCLError(true), options.ParserWithDebug(debug)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) @@ -98,9 +92,6 @@ resource "problem" "uhoh" { require.NoError(t, err) testutil.AssertRuleFound(t, badRule.LongID(), results, "") - if t.Failed() { - fmt.Println(debug.String()) - } } func Test_ProblemInModuleInSiblingDir(t *testing.T) { @@ -293,7 +284,7 @@ resource "problem" "uhoh" { `, }) - p := parser.New(fs, "", parser.OptionStopOnHCLError(true), options.ParserWithDebug(os.Stderr)) + p := parser.New(fs, "", parser.OptionStopOnHCLError(true)) err := p.ParseFS(context.TODO(), "project") require.NoError(t, err) modules, _, err := p.EvaluateAll(context.TODO()) diff --git a/pkg/iac/scanners/terraform/options.go b/pkg/iac/scanners/terraform/options.go index f5a0d2223534..d256f0a34daa 100644 --- a/pkg/iac/scanners/terraform/options.go +++ b/pkg/iac/scanners/terraform/options.go @@ -14,7 +14,7 @@ type ConfigurableTerraformScanner interface { options.ConfigurableScanner SetForceAllDirs(bool) AddExecutorOptions(options ...executor.Option) - AddParserOptions(options ...options.ParserOption) + AddParserOptions(options ...parser.Option) } func ScannerWithTFVarsPaths(paths ...string) options.ScannerOption { diff --git a/pkg/iac/scanners/terraform/parser/evaluator.go b/pkg/iac/scanners/terraform/parser/evaluator.go index e0231d2311a5..811f618787b5 100644 --- a/pkg/iac/scanners/terraform/parser/evaluator.go +++ b/pkg/iac/scanners/terraform/parser/evaluator.go @@ -13,11 +13,11 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/convert" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/ignore" "github.com/aquasecurity/trivy/pkg/iac/terraform" tfcontext "github.com/aquasecurity/trivy/pkg/iac/terraform/context" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) const ( @@ -25,6 +25,7 @@ const ( ) type evaluator struct { + logger *log.Logger filesystem fs.FS ctx *tfcontext.Context blocks terraform.Blocks @@ -35,7 +36,6 @@ type evaluator struct { moduleName string ignores ignore.Rules parentParser *Parser - debug debug.Logger allowDownloads bool skipCachedModules bool } @@ -52,7 +52,7 @@ func newEvaluator( moduleMetadata *modulesMetadata, workspace string, ignores ignore.Rules, - logger debug.Logger, + logger *log.Logger, allowDownloads bool, skipCachedModules bool, ) *evaluator { @@ -84,7 +84,7 @@ func newEvaluator( inputVars: inputVars, moduleMetadata: moduleMetadata, ignores: ignores, - debug: logger, + logger: logger, allowDownloads: allowDownloads, skipCachedModules: skipCachedModules, } @@ -114,20 +114,24 @@ func (e *evaluator) exportOutputs() cty.Value { continue } data[block.Label()] = attr.Value() - e.debug.Log("Added module output %s=%s.", block.Label(), attr.Value().GoString()) + e.logger.Debug( + "Added module output", + log.String("block", block.Label()), + log.String("value", attr.Value().GoString()), + ) } return cty.ObjectVal(data) } 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) + e.logger.Debug("Starting module evaluation...", log.String("path", e.modulePath)) - fsMap := make(map[string]fs.FS) - fsMap[fsKey] = e.filesystem + fsKey := types.CreateFSKey(e.filesystem) + fsMap := map[string]fs.FS{ + fsKey: e.filesystem, + } - e.debug.Log("Starting module evaluation...") e.evaluateSteps() // expand out resources and modules via count, for-each and dynamic @@ -135,21 +139,37 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str e.blocks = e.expandBlocks(e.blocks) e.blocks = e.expandBlocks(e.blocks) - e.debug.Log("Starting submodule evaluation...") + submodules := e.evaluateSubmodules(ctx, fsMap) + + e.logger.Debug("Starting post-submodules evaluation...") + e.evaluateSteps() + + e.logger.Debug("Module evaluation complete.") + rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) + return append(terraform.Modules{rootModule}, submodules...), fsMap +} + +func (e *evaluator) evaluateSubmodules(ctx context.Context, fsMap map[string]fs.FS) terraform.Modules { submodules := e.loadSubmodules(ctx) + if len(submodules) == 0 { + return nil + } + + e.logger.Debug("Starting submodules evaluation...") + for i := 0; i < maxContextIterations; i++ { changed := false for _, sm := range submodules { changed = changed || e.evaluateSubmodule(ctx, sm) } if !changed { - e.debug.Log("All submodules are evaluated at i=%d", i) + e.logger.Debug("All submodules are evaluated", log.Int("loop", i)) break } } - e.debug.Log("Starting post-submodule evaluation...") + e.logger.Debug("Starting post-submodule evaluation...") e.evaluateSteps() var modules terraform.Modules @@ -158,11 +178,8 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str fsMap = lo.Assign(fsMap, sm.fsMap) } - e.debug.Log("Finished processing %d submodule(s).", len(modules)) - - e.debug.Log("Module evaluation complete.") - rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) - return append(terraform.Modules{rootModule}, modules...), fsMap + e.logger.Debug("Finished processing submodule(s).", log.Int("count", len(modules))) + return modules } type submodule struct { @@ -181,7 +198,7 @@ func (e *evaluator) loadSubmodules(ctx context.Context) []*submodule { if errors.Is(err, ErrNoFiles) { continue } else if err != nil { - e.debug.Log("Failed to load submodule '%s': %s.", definition.Name, err) + e.logger.Error("Failed to load submodule", log.String("name", definition.Name), log.Err(err)) continue } @@ -199,12 +216,12 @@ func (e *evaluator) evaluateSubmodule(ctx context.Context, sm *submodule) bool { inputVars := sm.definition.inputVars() if len(sm.modules) > 0 { if reflect.DeepEqual(inputVars, sm.lastState) { - e.debug.Log("Submodule %s inputs unchanged", sm.definition.Name) + e.logger.Debug("Submodule inputs unchanged", log.String("name", sm.definition.Name)) return false } } - e.debug.Log("Evaluating submodule %s", sm.definition.Name) + e.logger.Debug("Evaluating submodule", log.String("name", sm.definition.Name)) sm.eval.inputVars = inputVars sm.modules, sm.fsMap = sm.eval.EvaluateAll(ctx) outputs := sm.eval.exportOutputs() @@ -223,12 +240,12 @@ func (e *evaluator) evaluateSteps() { var lastContext hcl.EvalContext for i := 0; i < maxContextIterations; i++ { - e.debug.Log("Starting iteration %d", i) + e.logger.Debug("Starting iteration", log.Int("iteration", i)) e.evaluateStep() // if ctx matches the last evaluation, we can bail, nothing left to resolve if i > 0 && reflect.DeepEqual(lastContext.Variables, e.ctx.Inner().Variables) { - e.debug.Log("Context unchanged at i=%d", i) + e.logger.Debug("Context unchanged", log.Int("iteration", i)) break } if len(e.ctx.Inner().Variables) != len(lastContext.Variables) { @@ -283,6 +300,7 @@ func isBlockSupportsForEachMetaArgument(block *terraform.Block) bool { } func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool) terraform.Blocks { + var forEachFiltered terraform.Blocks for _, block := range blocks { @@ -297,6 +315,10 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool forEachVal := forEachAttr.Value() if forEachVal.IsNull() || !forEachVal.IsKnown() || !forEachAttr.IsIterable() { + e.logger.Error(`Failed to expand block. Invalid "for-each" argument. Must be known and iterable.`, + log.String("block", block.FullName()), + log.String("value", forEachVal.GoString()), + ) continue } @@ -310,9 +332,12 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool // instances are identified by a map key (or set member) from the value provided to for_each idx, err := convert.Convert(key, cty.String) if err != nil { - e.debug.Log( - `Invalid "for-each" argument: map key (or set value) is not a string, but %s`, - key.Type().FriendlyName(), + e.logger.Error( + `Failed to expand block. Invalid "for-each" argument: map key (or set value) is not a string`, + log.String("block", block.FullName()), + log.String("key", key.GoString()), + log.String("value", val.GoString()), + log.Err(err), ) return } @@ -324,7 +349,13 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool !forEachVal.Type().IsMapType() && !isDynamic { stringVal, err := convert.Convert(val, cty.String) if err != nil { - e.debug.Log("Failed to convert for-each arg %v to string", val) + e.logger.Error( + "Failed to expand block. Invalid 'for-each' argument: value is not a string", + log.String("block", block.FullName()), + log.String("key", idx.AsString()), + log.String("value", val.GoString()), + log.Err(err), + ) return } idx = stringVal @@ -349,7 +380,8 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool ctx.Set(idx, refs[0].TypeLabel(), "key") ctx.Set(val, refs[0].TypeLabel(), "value") } else { - e.debug.Log("Ignoring iterator attribute in dynamic block, expected one reference but got %d", len(refs)) + e.logger.Debug("Ignoring iterator attribute in dynamic block, expected one reference", + log.Int("refs", len(refs))) } } } @@ -367,7 +399,10 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool // So we must replace the old resource with a map with the attributes of the resource. e.ctx.Replace(cty.ObjectVal(clones), metadata.Reference()) } - e.debug.Log("Expanded block '%s' into %d clones via 'for_each' attribute.", block.LocalName(), len(clones)) + e.logger.Debug("Expanded block into clones via 'for_each' attribute.", + log.String("block", block.FullName()), + log.Int("clones", len(clones)), + ) } return forEachFiltered @@ -409,7 +444,11 @@ func (e *evaluator) expandBlockCounts(blocks terraform.Blocks) terraform.Blocks } else { e.ctx.SetByDot(cty.TupleVal(clones), metadata.Reference()) } - e.debug.Log("Expanded block '%s' into %d clones via 'count' attribute.", block.LocalName(), len(clones)) + e.logger.Debug( + "Expanded block into clones via 'count' attribute.", + log.String("block", block.FullName()), + log.Int("clones", len(clones)), + ) } return countFiltered diff --git a/pkg/iac/scanners/terraform/parser/load_module.go b/pkg/iac/scanners/terraform/parser/load_module.go index 0bd6a6395936..78ebe3430b4e 100644 --- a/pkg/iac/scanners/terraform/parser/load_module.go +++ b/pkg/iac/scanners/terraform/parser/load_module.go @@ -11,6 +11,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser/resolvers" "github.com/aquasecurity/trivy/pkg/iac/terraform" + "github.com/aquasecurity/trivy/pkg/log" ) type ModuleDefinition struct { @@ -42,11 +43,11 @@ func (e *evaluator) loadModules(ctx context.Context) []*ModuleDefinition { } moduleDefinition, err := e.loadModule(ctx, moduleBlock) if err != nil { - e.debug.Log("Failed to load module %q. Maybe try 'terraform init'?", err) + e.logger.Error("Failed to load module. Maybe try 'terraform init'?", log.Err(err)) continue } - e.debug.Log("Loaded module %q from %q.", moduleDefinition.Name, moduleDefinition.Path) + e.logger.Debug("Loaded module", log.String("name", moduleDefinition.Name), log.FilePath(moduleDefinition.Path)) moduleDefinitions = append(moduleDefinitions, moduleDefinition) } @@ -77,7 +78,7 @@ func (e *evaluator) loadModule(ctx context.Context, b *terraform.Block) (*Module } if def, err := e.loadModuleFromTerraformCache(ctx, b, source); err == nil { - e.debug.Log("found module '%s' in .terraform/modules", source) + e.logger.Debug("Using module from Terraform cache .terraform/modules", log.String("source", source)) return def, nil } @@ -110,7 +111,11 @@ func (e *evaluator) loadModuleFromTerraformCache(ctx context.Context, b *terrafo } } - e.debug.Log("Module '%s' resolved to path '%s' in filesystem '%s' using modules.json", b.FullName(), modulePath, e.filesystem) + e.logger.Debug("Module resolved using modules.json", + log.String("block", b.FullName()), + log.String("source", source), + log.String("modulePath", modulePath), + ) moduleParser := e.parentParser.newModuleParser(e.filesystem, source, modulePath, b.Label(), b) if err := moduleParser.ParseFS(ctx, modulePath); err != nil { return nil, err @@ -126,7 +131,7 @@ func (e *evaluator) loadModuleFromTerraformCache(ctx context.Context, b *terrafo func (e *evaluator) loadExternalModule(ctx context.Context, b *terraform.Block, source string) (*ModuleDefinition, error) { - e.debug.Log("locating non-initialized module '%s'...", source) + e.logger.Debug("Locating non-initialized module", log.String("source", source)) version := b.GetAttribute("version").AsStringValueOrDefault("", b).Value() opt := resolvers.Options{ @@ -137,7 +142,7 @@ func (e *evaluator) loadExternalModule(ctx context.Context, b *terraform.Block, WorkingDir: e.projectRootPath, Name: b.FullName(), ModulePath: e.modulePath, - DebugLogger: e.debug.Extend("resolver"), + Logger: log.WithPrefix("module resolver"), AllowDownloads: e.allowDownloads, SkipCache: e.skipCachedModules, } @@ -147,7 +152,12 @@ func (e *evaluator) loadExternalModule(ctx context.Context, b *terraform.Block, return nil, err } prefix = path.Join(e.parentParser.moduleSource, prefix) - e.debug.Log("Module '%s' resolved to path '%s' in filesystem '%s' with prefix '%s'", b.FullName(), downloadPath, filesystem, prefix) + e.logger.Debug("Module resolved", + log.String("block", b.FullName()), + log.String("source", source), + log.String("prefix", prefix), + log.FilePath(downloadPath), + ) moduleParser := e.parentParser.newModuleParser(filesystem, prefix, downloadPath, b.Label(), b) if err := moduleParser.ParseFS(ctx, downloadPath); err != nil { return nil, err diff --git a/pkg/iac/scanners/terraform/parser/module_retrieval.go b/pkg/iac/scanners/terraform/parser/module_retrieval.go index 165f64eef1cb..cc374b68eeb7 100644 --- a/pkg/iac/scanners/terraform/parser/module_retrieval.go +++ b/pkg/iac/scanners/terraform/parser/module_retrieval.go @@ -6,6 +6,7 @@ import ( "io/fs" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser/resolvers" + "github.com/aquasecurity/trivy/pkg/log" ) type ModuleResolver interface { @@ -20,12 +21,13 @@ var defaultResolvers = []ModuleResolver{ } func resolveModule(ctx context.Context, current fs.FS, opt resolvers.Options) (filesystem fs.FS, sourcePrefix, downloadPath string, err error) { - opt.Debug("Resolving module '%s' with source: '%s'...", opt.Name, opt.Source) + opt.Logger.Debug("Resolving module", + log.String("name", opt.Name), log.String("source", opt.Source)) for _, resolver := range defaultResolvers { if filesystem, prefix, path, applies, err := resolver.Resolve(ctx, current, opt); err != nil { return nil, "", "", err } else if applies { - opt.Debug("Module path is %s", path) + opt.Logger.Debug("Module resolved", log.FilePath(path)) return filesystem, prefix, path, nil } } diff --git a/pkg/iac/scanners/terraform/parser/option.go b/pkg/iac/scanners/terraform/parser/option.go index 76146a4ea6bf..277cf2a58c61 100644 --- a/pkg/iac/scanners/terraform/parser/option.go +++ b/pkg/iac/scanners/terraform/parser/option.go @@ -4,75 +4,48 @@ import ( "io/fs" "github.com/zclconf/go-cty/cty" - - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" ) -type ConfigurableTerraformParser interface { - options.ConfigurableParser - SetTFVarsPaths(...string) - SetTFVars(vars map[string]cty.Value) - SetStopOnHCLError(bool) - SetWorkspaceName(string) - SetAllowDownloads(bool) - SetSkipCachedModules(bool) - SetConfigsFS(fsys fs.FS) -} - -type Option func(p ConfigurableTerraformParser) +type Option func(p *Parser) -func OptionWithTFVarsPaths(paths ...string) options.ParserOption { - return func(p options.ConfigurableParser) { - if tf, ok := p.(ConfigurableTerraformParser); ok { - tf.SetTFVarsPaths(paths...) - } +func OptionWithTFVarsPaths(paths ...string) Option { + return func(p *Parser) { + p.tfvarsPaths = paths } } -func OptionsWithTfVars(vars map[string]cty.Value) options.ParserOption { - return func(p options.ConfigurableParser) { - if tf, ok := p.(ConfigurableTerraformParser); ok { - tf.SetTFVars(vars) - } +func OptionStopOnHCLError(stop bool) Option { + return func(p *Parser) { + p.stopOnHCLError = stop } } -func OptionStopOnHCLError(stop bool) options.ParserOption { - return func(p options.ConfigurableParser) { - if tf, ok := p.(ConfigurableTerraformParser); ok { - tf.SetStopOnHCLError(stop) - } +func OptionsWithTfVars(vars map[string]cty.Value) Option { + return func(p *Parser) { + p.tfvars = vars } } -func OptionWithWorkspaceName(workspaceName string) options.ParserOption { - return func(p options.ConfigurableParser) { - if tf, ok := p.(ConfigurableTerraformParser); ok { - tf.SetWorkspaceName(workspaceName) - } +func OptionWithWorkspaceName(workspaceName string) Option { + return func(p *Parser) { + p.workspaceName = workspaceName } } -func OptionWithDownloads(allowed bool) options.ParserOption { - return func(p options.ConfigurableParser) { - if tf, ok := p.(ConfigurableTerraformParser); ok { - tf.SetAllowDownloads(allowed) - } +func OptionWithDownloads(allowed bool) Option { + return func(p *Parser) { + p.allowDownloads = allowed } } -func OptionWithSkipCachedModules(b bool) options.ParserOption { - return func(p options.ConfigurableParser) { - if tf, ok := p.(ConfigurableTerraformParser); ok { - tf.SetSkipCachedModules(b) - } +func OptionWithSkipCachedModules(b bool) Option { + return func(p *Parser) { + p.skipCachedModules = b } } -func OptionWithConfigsFS(fsys fs.FS) options.ParserOption { - return func(s options.ConfigurableParser) { - if p, ok := s.(ConfigurableTerraformParser); ok { - p.SetConfigsFS(fsys) - } +func OptionWithConfigsFS(fsys fs.FS) Option { + return func(p *Parser) { + p.configsFS = fsys } } diff --git a/pkg/iac/scanners/terraform/parser/parser.go b/pkg/iac/scanners/terraform/parser/parser.go index a5b2b909a462..60940d6c6241 100644 --- a/pkg/iac/scanners/terraform/parser/parser.go +++ b/pkg/iac/scanners/terraform/parser/parser.go @@ -15,11 +15,10 @@ import ( "github.com/hashicorp/hcl/v2/hclparse" "github.com/zclconf/go-cty/cty" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/ignore" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/terraform" tfcontext "github.com/aquasecurity/trivy/pkg/iac/terraform/context" + "github.com/aquasecurity/trivy/pkg/log" ) type sourceFile struct { @@ -27,8 +26,6 @@ type sourceFile struct { path string } -var _ ConfigurableTerraformParser = (*Parser)(nil) - // Parser is a tool for parsing terraform templates at a given file system location type Parser struct { projectRoot string @@ -44,48 +41,16 @@ type Parser struct { workspaceName string underlying *hclparse.Parser children []*Parser - options []options.ParserOption - debug debug.Logger + options []Option + logger *log.Logger allowDownloads bool skipCachedModules bool fsMap map[string]fs.FS configsFS fs.FS } -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "terraform", "parser", "<"+p.moduleName+">") -} - -func (p *Parser) SetTFVarsPaths(s ...string) { - p.tfvarsPaths = s -} - -func (p *Parser) SetTFVars(vars map[string]cty.Value) { - p.tfvars = vars -} - -func (p *Parser) SetStopOnHCLError(b bool) { - p.stopOnHCLError = b -} - -func (p *Parser) SetWorkspaceName(s string) { - p.workspaceName = s -} - -func (p *Parser) SetAllowDownloads(b bool) { - p.allowDownloads = b -} - -func (p *Parser) SetSkipCachedModules(b bool) { - p.skipCachedModules = b -} - -func (p *Parser) SetConfigsFS(fsys fs.FS) { - p.configsFS = fsys -} - // New creates a new Parser -func New(moduleFS fs.FS, moduleSource string, opts ...options.ParserOption) *Parser { +func New(moduleFS fs.FS, moduleSource string, opts ...Option) *Parser { p := &Parser{ workspaceName: "default", underlying: hclparse.NewParser(), @@ -95,6 +60,7 @@ func New(moduleFS fs.FS, moduleSource string, opts ...options.ParserOption) *Par moduleFS: moduleFS, moduleSource: moduleSource, configsFS: moduleFS, + logger: log.WithPrefix("terraform parser").With("module", "root"), tfvars: make(map[string]cty.Value), } @@ -110,6 +76,7 @@ func (p *Parser) newModuleParser(moduleFS fs.FS, moduleSource, modulePath, modul mp.modulePath = modulePath mp.moduleBlock = moduleBlock mp.moduleName = moduleName + mp.logger = log.WithPrefix("terraform parser").With("module", moduleName) mp.projectRoot = p.projectRoot p.children = append(p.children, mp) for _, option := range p.options { @@ -126,7 +93,7 @@ func (p *Parser) ParseFile(_ context.Context, fullPath string) error { return nil } - p.debug.Log("Parsing '%s'...", fullPath) + p.logger.Debug("Parsing", log.FilePath(fullPath)) f, err := p.moduleFS.Open(filepath.ToSlash(fullPath)) if err != nil { return err @@ -139,7 +106,7 @@ func (p *Parser) ParseFile(_ context.Context, fullPath string) error { } if dir := path.Dir(fullPath); p.projectRoot == "" { - p.debug.Log("Setting project/module root to '%s'", dir) + p.logger.Debug("Setting project/module root", log.FilePath(dir)) p.projectRoot = dir p.modulePath = dir } @@ -160,7 +127,7 @@ func (p *Parser) ParseFile(_ context.Context, fullPath string) error { path: fullPath, }) - p.debug.Log("Added file %s.", fullPath) + p.logger.Debug("Added file", log.FilePath(fullPath)) return nil } @@ -170,13 +137,13 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) error { dir = path.Clean(dir) if p.projectRoot == "" { - p.debug.Log("Setting project/module root to '%s'", dir) + p.logger.Debug("Setting project/module root", log.FilePath(dir)) p.projectRoot = dir p.modulePath = dir } slashed := filepath.ToSlash(dir) - p.debug.Log("Parsing FS from '%s'", slashed) + p.logger.Debug("Parsing FS", log.FilePath(slashed)) fileInfos, err := fs.ReadDir(p.moduleFS, slashed) if err != nil { return err @@ -196,7 +163,7 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) error { if p.stopOnHCLError { return err } - p.debug.Log("error parsing '%s': %s", path, err) + p.logger.Error("Error parsing file", log.FilePath(path), log.Err(err)) continue } } @@ -207,10 +174,10 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) error { var ErrNoFiles = errors.New("no files found") func (p *Parser) Load(ctx context.Context) (*evaluator, error) { - p.debug.Log("Evaluating module...") + p.logger.Debug("Loading module", log.String("module", p.moduleName)) if len(p.files) == 0 { - p.debug.Log("No files found, nothing to do.") + p.logger.Info("No files found, nothing to do.") return nil, ErrNoFiles } @@ -218,31 +185,43 @@ func (p *Parser) Load(ctx context.Context) (*evaluator, error) { if err != nil { return nil, err } - 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.logger.Debug("Read block(s) and ignore(s)", + log.Int("blocks", len(blocks)), log.Int("ignores", len(ignores))) var inputVars map[string]cty.Value switch { case p.moduleBlock != nil: inputVars = p.moduleBlock.Values().AsValueMap() - p.debug.Log("Added %d input variables from module definition.", len(inputVars)) + p.logger.Debug("Added input variables from module definition", + log.Int("count", len(inputVars))) case len(p.tfvars) > 0: inputVars = p.tfvars - p.debug.Log("Added %d input variables from tfvars.", len(inputVars)) + p.logger.Debug("Added input variables from tfvars.", log.Int("count", len(inputVars))) default: inputVars, err = loadTFVars(p.configsFS, p.tfvarsPaths) if err != nil { return nil, err } - p.debug.Log("Added %d variables from tfvars.", len(inputVars)) + p.logger.Debug("Added input variables from tfvars", log.Int("count", len(inputVars))) + + if missingVars := missingVariableValues(blocks, inputVars); len(missingVars) > 0 { + p.logger.Warn( + "Variable values was not found in the environment or variable files. Evaluating may not work correctly.", + log.String("variables", strings.Join(missingVars, ", ")), + ) + } } modulesMetadata, metadataPath, err := loadModuleMetadata(p.moduleFS, p.projectRoot) if err != nil && !errors.Is(err, os.ErrNotExist) { - p.debug.Log("Error loading module metadata: %s.", err) + p.logger.Error("Error loading module metadata", log.Err(err)) } else if err == nil { - p.debug.Log("Loaded module metadata for %d module(s) from %q.", len(modulesMetadata.Modules), metadataPath) + p.logger.Debug("Loaded module metadata for modules", + log.FilePath(metadataPath), + log.Int("count", len(modulesMetadata.Modules)), + ) } workingDir, err := os.Getwd() @@ -250,7 +229,7 @@ func (p *Parser) Load(ctx context.Context) (*evaluator, error) { return nil, err } - p.debug.Log("Working directory for module evaluation is %q", workingDir) + p.logger.Debug("Working directory for module evaluation", log.FilePath(workingDir)) return newEvaluator( p.moduleFS, p, @@ -263,12 +242,25 @@ func (p *Parser) Load(ctx context.Context) (*evaluator, error) { modulesMetadata, p.workspaceName, ignores, - p.debug.Extend("evaluator"), + log.WithPrefix("terraform evaluator"), p.allowDownloads, p.skipCachedModules, ), nil } +func missingVariableValues(blocks terraform.Blocks, inputVars map[string]cty.Value) []string { + var missing []string + for _, varBlock := range blocks.OfType("variable") { + if varBlock.GetAttribute("default") == nil { + if _, ok := inputVars[varBlock.TypeLabel()]; !ok { + missing = append(missing, varBlock.TypeLabel()) + } + } + } + + return missing +} + func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, error) { e, err := p.Load(ctx) @@ -279,7 +271,7 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, } modules, fsMap := e.EvaluateAll(ctx) - p.debug.Log("Finished parsing module '%s'.", p.moduleName) + p.logger.Debug("Finished parsing module") p.fsMap = fsMap return modules, e.exportOutputs(), nil } @@ -301,7 +293,7 @@ func (p *Parser) readBlocks(files []sourceFile) (terraform.Blocks, ignore.Rules, if p.stopOnHCLError { return nil, nil, err } - p.debug.Log("Encountered HCL parse error: %s", err) + p.logger.Error("Encountered HCL parse error", log.FilePath(file.path), log.Err(err)) continue } for _, fileBlock := range fileBlocks { diff --git a/pkg/iac/scanners/terraform/parser/parser_test.go b/pkg/iac/scanners/terraform/parser/parser_test.go index df06dd2aa393..e3bd817748f6 100644 --- a/pkg/iac/scanners/terraform/parser/parser_test.go +++ b/pkg/iac/scanners/terraform/parser/parser_test.go @@ -1,7 +1,9 @@ package parser import ( + "bytes" "context" + "log/slog" "os" "path/filepath" "sort" @@ -13,8 +15,8 @@ import ( "github.com/zclconf/go-cty/cty" "github.com/aquasecurity/trivy/internal/testutil" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/terraform" + "github.com/aquasecurity/trivy/pkg/log" ) func Test_BasicParsing(t *testing.T) { @@ -172,7 +174,7 @@ output "mod_result" { `, }) - parser := New(fs, "", OptionStopOnHCLError(true), options.ParserWithDebug(os.Stderr)) + parser := New(fs, "", OptionStopOnHCLError(true)) require.NoError(t, parser.ParseFS(context.TODO(), "code")) modules, _, err := parser.EvaluateAll(context.TODO()) @@ -1847,3 +1849,38 @@ resource "something" "blah" { assert.True(t, r2.IsResolvable()) assert.Equal(t, "us-east-2", r2.Value().AsString()) } + +func TestLogAboutMissingVariableValues(t *testing.T) { + var buf bytes.Buffer + slog.SetDefault(slog.New(log.NewHandler(&buf, nil))) + + fsys := fstest.MapFS{ + "main.tf": &fstest.MapFile{ + Data: []byte(` +variable "foo" {} + +variable "bar" { + default = "bar" +} + +variable "baz" {} +`), + }, + "main.tfvars": &fstest.MapFile{ + Data: []byte(`baz = "baz"`), + }, + } + + parser := New( + fsys, "", + OptionStopOnHCLError(true), + OptionWithTFVarsPaths("main.tfvars"), + ) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + + _, err := parser.Load(context.TODO()) + require.NoError(t, err) + + assert.Contains(t, buf.String(), "Variable values was not found in the environment or variable files.") + assert.Contains(t, buf.String(), "variables=\"foo\"") +} diff --git a/pkg/iac/scanners/terraform/parser/resolvers/cache.go b/pkg/iac/scanners/terraform/parser/resolvers/cache.go index c8b2f660ed33..24f803f60139 100644 --- a/pkg/iac/scanners/terraform/parser/resolvers/cache.go +++ b/pkg/iac/scanners/terraform/parser/resolvers/cache.go @@ -8,6 +8,8 @@ import ( "io/fs" "os" "path/filepath" + + "github.com/aquasecurity/trivy/pkg/log" ) type cacheResolver struct{} @@ -42,21 +44,21 @@ func locateCacheDir(cacheDir string) (string, error) { func (r *cacheResolver) Resolve(_ context.Context, _ fs.FS, opt Options) (filesystem fs.FS, prefix, downloadPath string, applies bool, err error) { if opt.SkipCache { - opt.Debug("Cache is disabled.") + opt.Logger.Debug("Module caching is disabled") return nil, "", "", false, nil } cacheFS, err := locateCacheFS(opt.CacheDir) if err != nil { - opt.Debug("No cache filesystem is available on this machine.") + opt.Logger.Debug("No cache filesystem is available on this machine.", log.Err(err)) return nil, "", "", false, nil } src, subdir := splitPackageSubdirRaw(opt.Source) key := cacheKey(src, opt.Version) - opt.Debug("Trying to resolve: %s", key) + opt.Logger.Debug("Trying to resolve module via cache", log.String("key", key)) if info, err := fs.Stat(cacheFS, filepath.ToSlash(key)); err == nil && info.IsDir() { - opt.Debug("Module '%s' resolving via cache...", opt.Name) + opt.Logger.Debug("Module resolved from cache", log.String("key", key)) cacheDir, err := locateCacheDir(opt.CacheDir) if err != nil { return nil, "", "", true, err diff --git a/pkg/iac/scanners/terraform/parser/resolvers/local.go b/pkg/iac/scanners/terraform/parser/resolvers/local.go index eb053741c8ab..a11a294edcfb 100644 --- a/pkg/iac/scanners/terraform/parser/resolvers/local.go +++ b/pkg/iac/scanners/terraform/parser/resolvers/local.go @@ -5,6 +5,8 @@ import ( "io/fs" "path" "path/filepath" + + "github.com/aquasecurity/trivy/pkg/log" ) type localResolver struct{} @@ -17,11 +19,15 @@ func (r *localResolver) Resolve(_ context.Context, target fs.FS, opt Options) (f } joined := path.Clean(path.Join(opt.ModulePath, opt.Source)) if _, err := fs.Stat(target, filepath.ToSlash(joined)); err == nil { - opt.Debug("Module '%s' resolved locally to %s", opt.Name, joined) + opt.Logger.Debug("Module resolved locally", + log.String("name", opt.Name), log.FilePath(joined), + ) return target, "", joined, true, nil } clean := path.Clean(opt.Source) - opt.Debug("Module '%s' resolved locally to %s", opt.Name, clean) + opt.Logger.Debug("Module resolved locally", + log.String("name", opt.Name), log.FilePath(clean), + ) return target, "", clean, true, nil } diff --git a/pkg/iac/scanners/terraform/parser/resolvers/options.go b/pkg/iac/scanners/terraform/parser/resolvers/options.go index cdfde6b01bcc..73fd39689e84 100644 --- a/pkg/iac/scanners/terraform/parser/resolvers/options.go +++ b/pkg/iac/scanners/terraform/parser/resolvers/options.go @@ -3,12 +3,12 @@ package resolvers import ( "strings" - "github.com/aquasecurity/trivy/pkg/iac/debug" + "github.com/aquasecurity/trivy/pkg/log" ) type Options struct { Source, OriginalSource, Version, OriginalVersion, WorkingDir, Name, ModulePath string - DebugLogger debug.Logger + Logger *log.Logger AllowDownloads bool SkipCache bool RelativePath string @@ -23,7 +23,3 @@ func (o *Options) hasPrefix(prefixes ...string) bool { } return false } - -func (o *Options) Debug(format string, args ...any) { - o.DebugLogger.Log(format, args...) -} diff --git a/pkg/iac/scanners/terraform/parser/resolvers/registry.go b/pkg/iac/scanners/terraform/parser/resolvers/registry.go index 8f2ab2ecdde1..471416463cad 100644 --- a/pkg/iac/scanners/terraform/parser/resolvers/registry.go +++ b/pkg/iac/scanners/terraform/parser/resolvers/registry.go @@ -14,6 +14,7 @@ import ( "golang.org/x/net/idna" "github.com/aquasecurity/go-version/pkg/version" + "github.com/aquasecurity/trivy/pkg/log" ) type registryResolver struct { @@ -59,9 +60,11 @@ func (r *registryResolver) Resolve(ctx context.Context, target fs.FS, opt Option token, err = getPrivateRegistryTokenFromEnvVars(hostname) if err == nil { - opt.Debug("Found a token for the registry at %s", hostname) + opt.Logger.Debug("Found a token for the registry", log.String("hostname", hostname)) } else { - opt.Debug(err.Error()) + opt.Logger.Error( + "Failed to find a token for the registry", + log.String("hostname", hostname), log.Err(err)) } } @@ -69,7 +72,8 @@ func (r *registryResolver) Resolve(ctx context.Context, target fs.FS, opt Option if opt.Version != "" { versionUrl := fmt.Sprintf("https://%s/v1/modules/%s/versions", hostname, moduleName) - opt.Debug("Requesting module versions from registry using '%s'...", versionUrl) + opt.Logger.Debug("Requesting module versions from registry using", + log.String("url", versionUrl)) req, err := http.NewRequestWithContext(ctx, http.MethodGet, versionUrl, nil) if err != nil { return nil, "", "", true, err @@ -94,7 +98,8 @@ func (r *registryResolver) Resolve(ctx context.Context, target fs.FS, opt Option if err != nil { return nil, "", "", true, err } - opt.Debug("Found version '%s' for constraint '%s'", opt.Version, inputVersion) + opt.Logger.Debug("Found module version", + log.String("version", opt.Version), log.String("constraint", inputVersion)) } var url string @@ -104,7 +109,7 @@ func (r *registryResolver) Resolve(ctx context.Context, target fs.FS, opt Option url = fmt.Sprintf("https://%s/v1/modules/%s/%s/download", hostname, moduleName, opt.Version) } - opt.Debug("Requesting module source from registry using '%s'...", url) + opt.Logger.Debug("Requesting module source from registry", log.String("url", url)) req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { @@ -145,7 +150,8 @@ func (r *registryResolver) Resolve(ctx context.Context, target fs.FS, opt Option return nil, "", "", true, fmt.Errorf("no source was found for the registry at %s", hostname) } - opt.Debug("Module '%s' resolved via registry to new source: '%s'", opt.Name, opt.Source) + opt.Logger.Debug("Module resolved via registry to new source", + log.String("source", opt.Source), log.String("name", moduleName)) filesystem, prefix, downloadPath, _, err = Remote.Resolve(ctx, target, opt) if err != nil { diff --git a/pkg/iac/scanners/terraform/parser/resolvers/remote.go b/pkg/iac/scanners/terraform/parser/resolvers/remote.go index 790b3509fd33..467f2cee6970 100644 --- a/pkg/iac/scanners/terraform/parser/resolvers/remote.go +++ b/pkg/iac/scanners/terraform/parser/resolvers/remote.go @@ -9,6 +9,8 @@ import ( "sync/atomic" "github.com/hashicorp/go-getter" + + "github.com/aquasecurity/trivy/pkg/log" ) type remoteResolver struct { @@ -20,9 +22,9 @@ var Remote = &remoteResolver{ } func (r *remoteResolver) incrementCount(o Options) { - o.Debug("Incrementing the download counter") + atomic.CompareAndSwapInt32(&r.count, r.count, r.count+1) - o.Debug("Download counter is now %d", r.count) + o.Logger.Debug("Incrementing the download counter", log.Int("count", int(r.count))) } func (r *remoteResolver) GetDownloadCount() int { @@ -40,7 +42,7 @@ func (r *remoteResolver) Resolve(ctx context.Context, _ fs.FS, opt Options) (fil src, subdir := splitPackageSubdirRaw(opt.OriginalSource) key := cacheKey(src, opt.OriginalVersion) - opt.Debug("Storing with cache key %s", key) + opt.Logger.Debug("Caching module", log.String("key", key)) baseCacheDir, err := locateCacheDir(opt.CacheDir) if err != nil { @@ -52,8 +54,10 @@ func (r *remoteResolver) Resolve(ctx context.Context, _ fs.FS, opt Options) (fil } r.incrementCount(opt) - opt.Debug("Successfully downloaded %s from %s", opt.Name, opt.Source) - opt.Debug("Module '%s' resolved via remote download.", opt.Name) + opt.Logger.Debug("Successfully resolve module via remote download", + log.String("name", opt.Name), + log.String("source", opt.Source), + ) return os.DirFS(cacheDir), opt.Source, subdir, true, nil } @@ -68,7 +72,7 @@ func (r *remoteResolver) download(ctx context.Context, opt Options, dst string) // Overwrite the file getter so that a file will be copied getter.Getters["file"] = &getter.FileGetter{Copy: true} - opt.Debug("Downloading %s...", opt.Source) + opt.Logger.Debug("Downloading module", log.String("source", opt.Source)) // Build the client client := &getter.Client{ diff --git a/pkg/iac/scanners/terraform/scanner.go b/pkg/iac/scanners/terraform/scanner.go index 5e7cea83abfd..080d26c0d155 100644 --- a/pkg/iac/scanners/terraform/scanner.go +++ b/pkg/iac/scanners/terraform/scanner.go @@ -11,7 +11,6 @@ import ( "strings" "sync" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" @@ -21,6 +20,7 @@ import ( "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/parser" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ scanners.FSScanner = (*Scanner)(nil) @@ -28,17 +28,18 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) var _ ConfigurableTerraformScanner = (*Scanner)(nil) type Scanner struct { - mu sync.Mutex - options []options.ScannerOption - parserOpt []options.ParserOption - executorOpt []executor.Option - dirs map[string]struct{} - forceAllDirs bool - policyDirs []string - policyReaders []io.Reader - regoScanner *rego.Scanner - execLock sync.RWMutex - debug debug.Logger + mu sync.Mutex + logger *log.Logger + options []options.ScannerOption + parserOpt []parser.Option + executorOpt []executor.Option + dirs map[string]struct{} + forceAllDirs bool + policyDirs []string + policyReaders []io.Reader + regoScanner *rego.Scanner + execLock sync.RWMutex + frameworks []framework.Framework spec string loadEmbeddedLibraries bool @@ -76,7 +77,7 @@ func (s *Scanner) SetForceAllDirs(b bool) { s.forceAllDirs = b } -func (s *Scanner) AddParserOptions(opts ...options.ParserOption) { +func (s *Scanner) AddParserOptions(opts ...parser.Option) { s.parserOpt = append(s.parserOpt, opts...) } @@ -88,12 +89,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.parserOpt = append(s.parserOpt, options.ParserWithDebug(writer)) - s.executorOpt = append(s.executorOpt, executor.OptionWithDebugWriter(writer)) - s.debug = debug.New(writer, "terraform", "scanner") -} - func (s *Scanner) SetTraceWriter(_ io.Writer) { } @@ -120,6 +115,7 @@ func New(opts ...options.ScannerOption) *Scanner { s := &Scanner{ dirs: make(map[string]struct{}), options: opts, + logger: log.WithPrefix("terraform scanner"), } for _, opt := range opts { opt(s) @@ -134,8 +130,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return s.regoScanner, nil } regoScanner := rego.NewScanner(types.SourceCloud, s.options...) - regoScanner.SetParentDebugLogger(s.debug) - if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } @@ -152,14 +146,14 @@ type terraformRootModule struct { func (s *Scanner) ScanFS(ctx context.Context, target fs.FS, dir string) (scan.Results, error) { - s.debug.Log("Scanning [%s] at '%s'...", target, dir) + s.logger.Debug("Scanning directory", log.FilePath(dir)) // find directories which directly contain tf files modulePaths := s.findModules(target, dir, dir) sort.Strings(modulePaths) if len(modulePaths) == 0 { - s.debug.Log("no modules found") + s.logger.Info("No modules found, skipping directory", log.FilePath(dir)) return nil, nil } @@ -185,7 +179,7 @@ func (s *Scanner) ScanFS(ctx context.Context, target fs.FS, dir string) (scan.Re // parse all root module directories for _, dir := range rootDirs { - s.debug.Log("Scanning root module '%s'...", dir) + s.logger.Info("Scanning root module", log.FilePath(dir)) p := parser.New(target, "", s.parserOpt...) @@ -295,7 +289,7 @@ func (s *Scanner) findModules(target fs.FS, scanDir string, dirs ...string) []st func (s *Scanner) isRootModule(target fs.FS, dir string) bool { files, err := fs.ReadDir(target, filepath.ToSlash(dir)) if err != nil { - s.debug.Log("failed to read dir '%s' from filesystem [%s]: %s", dir, target, err) + s.logger.Error("Failed to read dir", log.FilePath(dir), log.Err(err)) return false } for _, file := range files { diff --git a/pkg/iac/scanners/terraform/scanner_integration_test.go b/pkg/iac/scanners/terraform/scanner_integration_test.go index 6c60dd8a9f2c..098d6a91d450 100644 --- a/pkg/iac/scanners/terraform/scanner_integration_test.go +++ b/pkg/iac/scanners/terraform/scanner_integration_test.go @@ -1,9 +1,7 @@ package terraform import ( - "bytes" "context" - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -45,10 +43,7 @@ deny[res] { }`, }) - debugLog := bytes.NewBuffer([]byte{}) - scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithPolicyDirs("rules"), options.ScannerWithEmbeddedPolicies(false), @@ -62,10 +57,6 @@ deny[res] { require.NoError(t, err) assert.Len(t, results.GetPassed(), 1) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_ScanChildUseRemoteModule(t *testing.T) { @@ -109,10 +100,7 @@ deny[res] { }`, }) - debugLog := bytes.NewBuffer([]byte{}) - scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithPolicyDirs("rules"), options.ScannerWithEmbeddedPolicies(false), @@ -126,10 +114,6 @@ deny[res] { require.NoError(t, err) assert.Len(t, results.GetPassed(), 1) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_OptionWithSkipDownloaded(t *testing.T) { diff --git a/pkg/iac/scanners/terraform/scanner_test.go b/pkg/iac/scanners/terraform/scanner_test.go index 20e839ff91ab..5c85995027f4 100644 --- a/pkg/iac/scanners/terraform/scanner_test.go +++ b/pkg/iac/scanners/terraform/scanner_test.go @@ -1,7 +1,6 @@ package terraform import ( - "bytes" "context" "fmt" "strconv" @@ -11,31 +10,10 @@ import ( "github.com/stretchr/testify/require" "github.com/aquasecurity/trivy/internal/testutil" - "github.com/aquasecurity/trivy/pkg/iac/providers" - "github.com/aquasecurity/trivy/pkg/iac/rules" "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" - "github.com/aquasecurity/trivy/pkg/iac/severity" - "github.com/aquasecurity/trivy/pkg/iac/terraform" ) -var alwaysFailRule = scan.Rule{ - Provider: providers.AWSProvider, - Service: "service", - ShortCode: "abc", - Severity: severity.High, - CustomChecks: scan.CustomChecks{ - Terraform: &scan.TerraformCustomCheck{ - RequiredTypes: []string{}, - RequiredLabels: []string{}, - Check: func(resourceBlock *terraform.Block, _ *terraform.Module) (results scan.Results) { - results.Add("oh no", resourceBlock) - return - }, - }, - }, -} - const emptyBucketRule = ` # METADATA # schemas: @@ -56,33 +34,6 @@ deny[res] { } ` -func scanWithOptions(t *testing.T, code string, opt ...options.ScannerOption) scan.Results { - - fs := testutil.CreateFS(t, map[string]string{ - "project/main.tf": code, - }) - - scanner := New(opt...) - results, err := scanner.ScanFS(context.TODO(), fs, "project") - require.NoError(t, err) - return results -} - -func Test_OptionWithDebugWriter(t *testing.T) { - reg := rules.Register(alwaysFailRule) - defer rules.Deregister(reg) - - buffer := bytes.NewBuffer([]byte{}) - - scannerOpts := []options.ScannerOption{ - options.ScannerWithDebug(buffer), - } - _ = scanWithOptions(t, ` -resource "something" "else" {} -`, scannerOpts...) - require.Positive(t, buffer.Len()) -} - func Test_OptionWithPolicyDirs(t *testing.T) { fs := testutil.CreateFS(t, map[string]string{ @@ -119,9 +70,7 @@ deny[cause] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithPolicyDirs("rules"), options.ScannerWithRegoOnly(true), @@ -168,10 +117,6 @@ deny[cause] { }, }, actualCode.Lines) - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } - } func Test_OptionWithPolicyNamespaces(t *testing.T) { @@ -278,7 +223,6 @@ cause := bucket.name } } assert.Equal(t, test.wantFailure, found) - }) } @@ -320,9 +264,7 @@ deny[cause] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithRegoOnly(true), ) @@ -332,10 +274,6 @@ deny[cause] { require.Len(t, results.GetFailed(), 1) assert.Equal(t, "AVD-TEST-0123", results[0].Rule().AVDID) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_OptionWithRegoOnly_CodeHighlighting(t *testing.T) { @@ -374,9 +312,7 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithRegoOnly(true), options.ScannerWithEmbeddedLibraries(true), @@ -388,10 +324,6 @@ deny[res] { require.Len(t, results.GetFailed(), 1) assert.Equal(t, "AVD-TEST-0123", results[0].Rule().AVDID) assert.NotNil(t, results[0].Metadata().Range().GetFS()) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_IAMPolicyRego(t *testing.T) { @@ -448,21 +380,12 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), - options.ScannerWithTrace(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithRegoOnly(true), options.ScannerWithEmbeddedLibraries(true), ) - defer func() { - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } - }() - results, err := scanner.ScanFS(context.TODO(), fs, "code") require.NoError(t, err) @@ -541,9 +464,7 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithRegoOnly(true), options.ScannerWithEmbeddedLibraries(true), @@ -556,9 +477,6 @@ deny[res] { assert.Equal(t, "AVD-TEST-0123", results[0].Rule().AVDID) assert.NotNil(t, results[0].Metadata().Range().GetFS()) - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_S3_Linking(t *testing.T) { @@ -603,10 +521,7 @@ resource "aws_s3_bucket_public_access_block" "foo" { "code/main.tf": code, }) - debugLog := bytes.NewBuffer([]byte{}) - scanner := New( - options.ScannerWithDebug(debugLog), - ) + scanner := New() results, err := scanner.ScanFS(context.TODO(), fs, "code") require.NoError(t, err) @@ -622,10 +537,6 @@ resource "aws_s3_bucket_public_access_block" "foo" { // versioning assert.NotEqual(t, "AVD-AWS-0090", result.Rule().AVDID) } - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_S3_Linking_PublicAccess(t *testing.T) { @@ -673,10 +584,7 @@ resource "aws_s3_bucket_public_access_block" "testB" { "code/main.tf": code, }) - debugLog := bytes.NewBuffer([]byte{}) - scanner := New( - options.ScannerWithDebug(debugLog), - ) + scanner := New() results, err := scanner.ScanFS(context.TODO(), fs, "code") require.NoError(t, err) @@ -686,9 +594,6 @@ resource "aws_s3_bucket_public_access_block" "testB" { assert.NotEqual(t, "AVD-AWS-0094", result.Rule().AVDID) } - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } // PoC for replacing Go with Rego: AVD-AWS-0001 @@ -732,9 +637,7 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithPolicyDirs("rules"), options.ScannerWithRegoOnly(true), @@ -788,11 +691,6 @@ deny[res] { Annotation: "", }, }, actualCode.Lines) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } - } func Test_OptionWithConfigsFileSystem(t *testing.T) { @@ -814,9 +712,7 @@ bucket_name = "test" `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithRegoOnly(true), @@ -832,10 +728,6 @@ bucket_name = "test" assert.Len(t, results, 1) assert.Len(t, results.GetPassed(), 1) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_OptionWithConfigsFileSystem_ConfigInCode(t *testing.T) { @@ -854,9 +746,7 @@ bucket_name = "test" `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithRegoOnly(true), @@ -872,10 +762,6 @@ bucket_name = "test" assert.Len(t, results, 1) assert.Len(t, results.GetPassed(), 1) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_DoNotScanNonRootModules(t *testing.T) { @@ -953,9 +839,7 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithPolicyDirs("rules"), options.ScannerWithEmbeddedPolicies(false), @@ -970,10 +854,6 @@ deny[res] { assert.Len(t, results.GetPassed(), 2) require.Len(t, results.GetFailed(), 1) assert.Equal(t, "AVD-AWS-0002", results.GetFailed()[0].Rule().AVDID) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func Test_RoleRefToOutput(t *testing.T) { @@ -1029,9 +909,7 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithRegoOnly(true), @@ -1100,9 +978,7 @@ deny[res] { }`, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithRegoOnly(true), @@ -1121,10 +997,6 @@ deny[res] { require.Len(t, results.GetPassed(), 1) assert.Equal(t, "AVD-AWS-0002", results.GetPassed()[0].Rule().AVDID) - - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } } func TestScanModuleWithCount(t *testing.T) { @@ -1170,9 +1042,7 @@ deny[res] { `, }) - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs("rules"), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithRegoOnly(true), diff --git a/pkg/iac/scanners/terraformplan/snapshot/scanner_test.go b/pkg/iac/scanners/terraformplan/snapshot/scanner_test.go index 6e7299e19dd1..c7ad3a3b4e0b 100644 --- a/pkg/iac/scanners/terraformplan/snapshot/scanner_test.go +++ b/pkg/iac/scanners/terraformplan/snapshot/scanner_test.go @@ -1,7 +1,6 @@ package snapshot import ( - "bytes" "context" "os" "path" @@ -108,9 +107,7 @@ func Test_ScanFS(t *testing.T) { t.Run(tc.dir, func(t *testing.T) { fs := os.DirFS("testdata") - debugLog := bytes.NewBuffer([]byte{}) scanner := New( - options.ScannerWithDebug(debugLog), options.ScannerWithPolicyDirs(path.Join(tc.dir, "checks")), options.ScannerWithPolicyFilesystem(fs), options.ScannerWithRegoOnly(true), diff --git a/pkg/iac/scanners/terraformplan/tfjson/parser/option.go b/pkg/iac/scanners/terraformplan/tfjson/parser/option.go deleted file mode 100644 index 09e09b97bed5..000000000000 --- a/pkg/iac/scanners/terraformplan/tfjson/parser/option.go +++ /dev/null @@ -1,17 +0,0 @@ -package parser - -import "io" - -type Option func(p *Parser) - -func OptionWithDebugWriter(w io.Writer) Option { - return func(p *Parser) { - p.debugWriter = w - } -} - -func OptionStopOnHCLError(stop bool) Option { - return func(p *Parser) { - p.stopOnHCLError = stop - } -} diff --git a/pkg/iac/scanners/terraformplan/tfjson/parser/parser.go b/pkg/iac/scanners/terraformplan/tfjson/parser/parser.go index 85d565bd307f..a55cfa99384f 100644 --- a/pkg/iac/scanners/terraformplan/tfjson/parser/parser.go +++ b/pkg/iac/scanners/terraformplan/tfjson/parser/parser.go @@ -11,28 +11,17 @@ import ( "github.com/liamg/memoryfs" "github.com/aquasecurity/trivy/pkg/iac/terraform" + "github.com/aquasecurity/trivy/pkg/log" ) type Parser struct { - debugWriter io.Writer - stopOnHCLError bool + logger *log.Logger } -func New(options ...Option) *Parser { - parser := &Parser{} - - for _, o := range options { - o(parser) +func New() *Parser { + return &Parser{ + logger: log.WithPrefix("tfjson parser"), } - return parser -} - -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debugWriter = writer -} - -func (p *Parser) SetStopOnHCLError(b bool) { - p.stopOnHCLError = b } func (p *Parser) ParseFile(filepath string) (*PlanFile, error) { diff --git a/pkg/iac/scanners/terraformplan/tfjson/scanner.go b/pkg/iac/scanners/terraformplan/tfjson/scanner.go index b390d4d10213..875644d1dd2a 100644 --- a/pkg/iac/scanners/terraformplan/tfjson/scanner.go +++ b/pkg/iac/scanners/terraformplan/tfjson/scanner.go @@ -8,13 +8,13 @@ import ( "github.com/bmatcuk/doublestar/v4" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" terraformScanner "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraform/executor" "github.com/aquasecurity/trivy/pkg/iac/scanners/terraformplan/tfjson/parser" + "github.com/aquasecurity/trivy/pkg/log" ) var tfPlanExts = []string{ @@ -23,10 +23,8 @@ var tfPlanExts = []string{ } type Scanner struct { - parser *parser.Parser - parserOpt []parser.Option - debug debug.Logger - + parser *parser.Parser + logger *log.Logger options []options.ScannerOption spec string executorOpt []executor.Option @@ -69,12 +67,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.parserOpt = append(s.parserOpt, parser.OptionWithDebugWriter(writer)) - s.executorOpt = append(s.executorOpt, executor.OptionWithDebugWriter(writer)) - s.debug = debug.New(writer, "tfplan", "scanner") -} - func (s *Scanner) SetTraceWriter(_ io.Writer) { } @@ -126,17 +118,19 @@ func (s *Scanner) ScanFS(ctx context.Context, inputFS fs.FS, dir string) (scan.R func New(opts ...options.ScannerOption) *Scanner { scanner := &Scanner{ options: opts, + logger: log.WithPrefix("tfjson scanner"), + parser: parser.New(), } for _, o := range opts { o(scanner) } - scanner.parser = parser.New(scanner.parserOpt...) + return scanner } func (s *Scanner) ScanFile(filepath string, fsys fs.FS) (scan.Results, error) { - s.debug.Log("Scanning file %s", filepath) + s.logger.Debug("Scanning file", log.FilePath(filepath)) file, err := fsys.Open(filepath) if err != nil { return nil, err diff --git a/pkg/iac/scanners/terraformplan/tfjson/scanner_test.go b/pkg/iac/scanners/terraformplan/tfjson/scanner_test.go index 8a878e206bb3..678b32e00f48 100644 --- a/pkg/iac/scanners/terraformplan/tfjson/scanner_test.go +++ b/pkg/iac/scanners/terraformplan/tfjson/scanner_test.go @@ -1,9 +1,7 @@ package tfjson import ( - "bytes" "context" - "fmt" "os" "testing" @@ -130,8 +128,7 @@ deny[cause] { "/rules/test.rego": tc.inputRego, }) - debugLog := bytes.NewBuffer([]byte{}) - so := append(tc.options, options.ScannerWithDebug(debugLog), options.ScannerWithPolicyFilesystem(fs)) + so := append(tc.options, options.ScannerWithPolicyFilesystem(fs)) scanner := New(so...) results, err := scanner.ScanFS(context.TODO(), fs, "code") @@ -142,9 +139,6 @@ deny[cause] { failure := results.GetFailed()[0] assert.Equal(t, "AVD-TEST-0123", failure.Rule().AVDID) - if t.Failed() { - fmt.Printf("Debug logs:\n%s\n", debugLog.String()) - } }) } } diff --git a/pkg/iac/scanners/toml/parser/parser.go b/pkg/iac/scanners/toml/parser/parser.go index 6beed83b1908..95444389d74d 100644 --- a/pkg/iac/scanners/toml/parser/parser.go +++ b/pkg/iac/scanners/toml/parser/parser.go @@ -2,33 +2,23 @@ package parser import ( "context" - "io" "io/fs" "path/filepath" "github.com/BurntSushi/toml" - "github.com/aquasecurity/trivy/pkg/iac/debug" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) -var _ options.ConfigurableParser = (*Parser)(nil) - type Parser struct { - debug debug.Logger -} - -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "toml", "parser") + logger *log.Logger } // New creates a new parser -func New(opts ...options.ParserOption) *Parser { - p := &Parser{} - for _, opt := range opts { - opt(p) +func New() *Parser { + return &Parser{ + logger: log.WithPrefix("toml parser"), } - return p } func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[string]any, error) { @@ -49,7 +39,7 @@ func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[st df, err := p.ParseFile(ctx, target, path) if err != nil { - p.debug.Log("Parse error in '%s': %s", path, err) + p.logger.Error("Parse error", log.FilePath(path), log.Err(err)) return nil } files[path] = df diff --git a/pkg/iac/scanners/toml/scanner.go b/pkg/iac/scanners/toml/scanner.go index b7dc3510da8f..235cc65dada5 100644 --- a/pkg/iac/scanners/toml/scanner.go +++ b/pkg/iac/scanners/toml/scanner.go @@ -6,22 +6,21 @@ import ( "io/fs" "sync" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/scanners/toml/parser" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex - debug debug.Logger + logger *log.Logger options []options.ScannerOption - parserOptions []options.ParserOption policyDirs []string policyReaders []io.Reader parser *parser.Parser @@ -61,11 +60,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "toml", "scanner") - s.parserOptions = append(s.parserOptions, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetTraceWriter(_ io.Writer) {} func (s *Scanner) SetPerResultTracingEnabled(_ bool) {} @@ -88,11 +82,12 @@ func (s *Scanner) SetRegoErrorLimit(_ int) {} func NewScanner(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("toml scanner"), } for _, opt := range opts { opt(s) } - s.parser = parser.New(s.parserOptions...) + s.parser = parser.New() return s } @@ -128,7 +123,7 @@ func (s *Scanner) ScanFile(ctx context.Context, fsys fs.FS, path string) (scan.R if err != nil { return nil, err } - s.debug.Log("Scanning %s...", path) + s.logger.Debug("Scanning", log.FilePath(path)) return s.scanRego(ctx, fsys, rego.Input{ Path: path, Contents: parsed, @@ -142,7 +137,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return s.regoScanner, nil } regoScanner := rego.NewScanner(types.SourceTOML, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } diff --git a/pkg/iac/scanners/yaml/parser/parser.go b/pkg/iac/scanners/yaml/parser/parser.go index febcfb100008..9b1f026ae248 100644 --- a/pkg/iac/scanners/yaml/parser/parser.go +++ b/pkg/iac/scanners/yaml/parser/parser.go @@ -9,33 +9,25 @@ import ( "gopkg.in/yaml.v3" - "github.com/aquasecurity/trivy/pkg/iac/debug" - "github.com/aquasecurity/trivy/pkg/iac/scanners/options" + "github.com/aquasecurity/trivy/pkg/log" ) -var _ options.ConfigurableParser = (*Parser)(nil) - type Parser struct { - debug debug.Logger -} - -func (p *Parser) SetDebugWriter(writer io.Writer) { - p.debug = debug.New(writer, "yaml", "parser") + logger *log.Logger } -// New creates a new parser -func New(opts ...options.ParserOption) *Parser { - p := &Parser{} - for _, opt := range opts { - opt(p) +// New creates a new YAML parser +func New() *Parser { + return &Parser{ + logger: log.WithPrefix("yaml parser"), } - return p } func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[string][]any, error) { files := make(map[string][]any) if err := fs.WalkDir(target, filepath.ToSlash(path), func(path string, entry fs.DirEntry, err error) error { + select { case <-ctx.Done(): return ctx.Err() @@ -50,7 +42,7 @@ func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) (map[st df, err := p.ParseFile(ctx, target, path) if err != nil { - p.debug.Log("Parse error in '%s': %s", path, err) + p.logger.Error("Parse error", log.FilePath(path), log.Err(err)) return nil } files[path] = df diff --git a/pkg/iac/scanners/yaml/scanner.go b/pkg/iac/scanners/yaml/scanner.go index 3ec508fdd6f5..67b661ee5971 100644 --- a/pkg/iac/scanners/yaml/scanner.go +++ b/pkg/iac/scanners/yaml/scanner.go @@ -6,13 +6,13 @@ import ( "io/fs" "sync" - "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" "github.com/aquasecurity/trivy/pkg/iac/rego" "github.com/aquasecurity/trivy/pkg/iac/scan" "github.com/aquasecurity/trivy/pkg/iac/scanners/options" "github.com/aquasecurity/trivy/pkg/iac/scanners/yaml/parser" "github.com/aquasecurity/trivy/pkg/iac/types" + "github.com/aquasecurity/trivy/pkg/log" ) var _ options.ConfigurableScanner = (*Scanner)(nil) @@ -20,8 +20,7 @@ var _ options.ConfigurableScanner = (*Scanner)(nil) type Scanner struct { mu sync.Mutex options []options.ScannerOption - parserOptions []options.ParserOption - debug debug.Logger + logger *log.Logger policyDirs []string policyReaders []io.Reader parser *parser.Parser @@ -61,11 +60,6 @@ func (s *Scanner) SetPolicyReaders(readers []io.Reader) { s.policyReaders = readers } -func (s *Scanner) SetDebugWriter(writer io.Writer) { - s.debug = debug.New(writer, "yaml", "scanner") - s.parserOptions = append(s.parserOptions, options.ParserWithDebug(writer)) -} - func (s *Scanner) SetTraceWriter(_ io.Writer) {} func (s *Scanner) SetPerResultTracingEnabled(_ bool) {} @@ -87,11 +81,12 @@ func (s *Scanner) SetRegoErrorLimit(_ int) {} func NewScanner(opts ...options.ScannerOption) *Scanner { s := &Scanner{ options: opts, + logger: log.WithPrefix("yaml scanner"), + parser: parser.New(), } for _, opt := range opts { opt(s) } - s.parser = parser.New(s.parserOptions...) return s } @@ -129,7 +124,7 @@ func (s *Scanner) ScanFile(ctx context.Context, fsys fs.FS, path string) (scan.R if err != nil { return nil, err } - s.debug.Log("Scanning %s...", path) + s.logger.Debug("Scanning", log.String("path", path)) return s.scanRego(ctx, fsys, rego.Input{ Path: path, Contents: parsed, @@ -143,7 +138,6 @@ func (s *Scanner) initRegoScanner(srcFS fs.FS) (*rego.Scanner, error) { return s.regoScanner, nil } regoScanner := rego.NewScanner(types.SourceYAML, s.options...) - regoScanner.SetParentDebugLogger(s.debug) if err := regoScanner.LoadPolicies(s.loadEmbeddedLibraries, s.loadEmbeddedPolicies, srcFS, s.policyDirs, s.policyReaders); err != nil { return nil, err } diff --git a/pkg/log/logger.go b/pkg/log/logger.go index 6eb8bde70dc2..73c4231fd4af 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -6,7 +6,6 @@ import ( "io" "log/slog" "os" - "strings" "github.com/samber/lo" ) @@ -83,18 +82,3 @@ func Fatal(msg string, args ...any) { slog.Default().Log(context.Background(), LevelFatal, msg, args...) os.Exit(1) } - -// WriteLogger is a wrapper around Logger to implement io.Writer -type WriteLogger struct { - logger *Logger -} - -// NewWriteLogger creates a new WriteLogger -func NewWriteLogger(logger *Logger) *WriteLogger { - return &WriteLogger{logger: logger} -} - -func (l *WriteLogger) Write(p []byte) (n int, err error) { - l.logger.Debug(strings.TrimSpace(string(p))) - return len(p), nil -} diff --git a/pkg/misconf/scanner.go b/pkg/misconf/scanner.go index 5737b3b8d3bc..2e979caba5e9 100644 --- a/pkg/misconf/scanner.go +++ b/pkg/misconf/scanner.go @@ -51,7 +51,6 @@ var enablediacTypes = map[detection.FileType]types.ConfigType{ } type ScannerOption struct { - Debug bool Trace bool RegoOnly bool Namespaces []string @@ -237,10 +236,6 @@ func scannerOptions(t detection.FileType, opt ScannerOption) ([]options.ScannerO options.ScannerWithCustomSchemas(schemas), ) - if opt.Debug { - opts = append(opts, options.ScannerWithDebug(log.NewWriteLogger(log.WithPrefix("misconf")))) - } - if opt.Trace { opts = append(opts, options.ScannerWithPerResultTracing(true)) }