chore(terraform): option to pass in instanced logger (#8738)

Author: Emyrk

pkg/iac/scanners/terraform/parser/load_module.go

@@ -143,7 +143,7 @@ func (e *evaluator) loadExternalModule(ctx context.Context, b *terraform.Block,
 		WorkingDir:      e.projectRootPath,
 		Name:            b.FullName(),
 		ModulePath:      e.modulePath,
-		Logger:          log.WithPrefix("module resolver"),
+		Logger:          e.logger.With(log.Prefix("module resolver")),
 		AllowDownloads:  e.allowDownloads,
 		SkipCache:       e.skipCachedModules,
 	}

pkg/iac/scanners/terraform/parser/option.go

@@ -4,6 +4,8 @@ import (
 	"io/fs"
 
 	"github.com/zclconf/go-cty/cty"
+
+	"github.com/aquasecurity/trivy/pkg/log"
 )
 
 type Option func(p *Parser)
@@ -20,6 +22,12 @@ func OptionStopOnHCLError(stop bool) Option {
 	}
 }
 
+func OptionWithLogger(log *log.Logger) Option {
+	return func(p *Parser) {
+		p.logger = log
+	}
+}
+
 func OptionsWithTfVars(vars map[string]cty.Value) Option {
 	return func(p *Parser) {
 		p.tfvars = vars

pkg/iac/scanners/terraform/parser/parser.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"io/fs"
+	"log/slog"
 	"os"
 	"path"
 	"path/filepath"
@@ -64,14 +65,17 @@ func New(moduleFS fs.FS, moduleSource string, opts ...Option) *Parser {
 		moduleFS:       moduleFS,
 		moduleSource:   moduleSource,
 		configsFS:      moduleFS,
-		logger:         log.WithPrefix("terraform parser").With("module", "root"),
+		logger:         slog.Default(),
 		tfvars:         make(map[string]cty.Value),
 	}
 
 	for _, option := range opts {
 		option(p)
 	}
 
+	// Scope the logger to the parser
+	p.logger = p.logger.With(log.Prefix("terraform parser")).With("module", "root")
+
 	return p
 }
 
@@ -80,14 +84,18 @@ 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.logger = p.logger
 	mp.projectRoot = p.projectRoot
 	mp.skipPaths = p.skipPaths
 	mp.options = p.options
 	p.children = append(p.children, mp)
 	for _, option := range p.options {
 		option(mp)
 	}
+
+	// The options above can reset the logger, so set the logging prefix after the
+	// options.
+	mp.logger = mp.logger.With(log.Prefix("terraform parser")).With("module", moduleName)
 	return mp
 }
 
@@ -303,7 +311,7 @@ func (p *Parser) Load(ctx context.Context) (*evaluator, error) {
 		modulesMetadata,
 		p.workspaceName,
 		ignores,
-		log.WithPrefix("terraform evaluator"),
+		p.logger.With(log.Prefix("terraform evaluator")),
 		p.allowDownloads,
 		p.skipCachedModules,
 	), nil

pkg/iac/scanners/terraform/parser/parser_test.go

@@ -2374,9 +2374,6 @@ variable "baz" {}
 }
 
 func TestLoadChildModulesFromLocalCache(t *testing.T) {
-	var buf bytes.Buffer
-	slog.SetDefault(slog.New(log.NewHandler(&buf, &log.Options{Level: log.LevelDebug})))
-
 	fsys := fstest.MapFS{
 		"main.tf": &fstest.MapFile{Data: []byte(`module "level_1" {
   source = "./modules/level_1"
@@ -2411,9 +2408,13 @@ func TestLoadChildModulesFromLocalCache(t *testing.T) {
 }`)},
 	}
 
+	var buf bytes.Buffer
+	logger := slog.New(log.NewHandler(&buf, &log.Options{Level: log.LevelDebug}))
+
 	parser := New(
 		fsys, "",
 		OptionStopOnHCLError(true),
+		OptionWithLogger(logger),
 	)
 	require.NoError(t, parser.ParseFS(t.Context(), "."))
 
@@ -2422,10 +2423,10 @@ func TestLoadChildModulesFromLocalCache(t *testing.T) {
 
 	assert.Len(t, modules, 5)
 
-	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tsource=\"./modules/level_1\"")
-	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tsource=\"../level_2\"")
-	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tsource=\"../level_3\"")
-	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tsource=\"../level_3\"")
+	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tmodule=\"root\" source=\"./modules/level_1\"")
+	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tmodule=\"level_1\" source=\"../level_2\"")
+	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tmodule=\"level_2\" source=\"../level_3\"")
+	assert.Contains(t, buf.String(), "Using module from Terraform cache .terraform/modules\tmodule=\"level_2\" source=\"../level_3\"")
 }
 
 func TestNilParser(t *testing.T) {
@@ -2633,3 +2634,118 @@ resource "foo" "bar" {
 	attr := foo.GetAttribute("attr")
 	assert.False(t, attr.IsResolvable())
 }
+
+// TestInstancedLogger checks if any global logs are generated by the terraform
+// parser + evaluation. All logs should be attached to the instanced logger. This
+// test does not run all error cases, so it will not capture all log output.
+func TestInstancedLogger(t *testing.T) {
+	// reset global logger
+	prevLog := slog.Default()
+	defer slog.SetDefault(prevLog)
+
+	// capture logs to the global logger
+	var buf bytes.Buffer
+
+	slog.SetDefault(slog.New(log.NewHandler(&buf, &log.Options{
+		Level: slog.LevelDebug,
+	})))
+
+	// create a new logger for the parser
+	var instance bytes.Buffer
+	logger := slog.New(log.NewHandler(&instance, &log.Options{
+		Level: slog.LevelDebug,
+	}))
+
+	opts := []Option{
+		OptionWithLogger(logger),
+		OptionStopOnHCLError(false),
+		OptionWithDownloads(false),
+	}
+	// Run some scenarios that will trigger logs
+	t.Run("ParserLogs", func(t *testing.T) {
+		fsys := fstest.MapFS{
+			"main.tf": &fstest.MapFile{
+				Data: []byte(`
+					bare words
+				`)},
+		}
+
+		parser := New(fsys, "", opts...)
+
+		// No error is returned, but some parser logs are expected
+		err := parser.ParseFS(t.Context(), ".")
+		require.NoError(t, err)
+		require.NotEmpty(t, instance.String())
+		instance.Reset()
+	})
+
+	t.Run("Evaluator", func(t *testing.T) {
+		fsys := fstest.MapFS{
+			"main.tf": &fstest.MapFile{
+				Data: []byte(`
+					locals {
+						phrase = "hello world"
+					}
+
+					output "phrase" {
+					  value = locals.phrase
+					}
+				`)},
+		}
+
+		parser := New(fsys, "", opts...)
+
+		err := parser.ParseFS(t.Context(), ".")
+		require.NoError(t, err)
+
+		_, err = parser.EvaluateAll(t.Context())
+		require.NoError(t, err)
+
+		require.NotEmpty(t, instance.String())
+		instance.Reset()
+	})
+
+	t.Run("ModuleFetching", func(t *testing.T) {
+		fsys := testutil.CreateFS(t, map[string]string{
+			"main.tf": `
+				module "invalid" {
+					source = "totally.invalid"
+				}
+
+				module "echo" {
+					source = "./echo"				
+					input = "hello"
+				}
+
+				locals {
+					foo = module.echo.value
+				}
+			`,
+			"echo/main.tf": `
+				variable "input" {
+					type = string				
+				}
+				output "value" {
+					value = var.input 
+				}
+			`,
+		})
+
+		parser := New(fsys, "", opts...)
+
+		err := parser.ParseFS(t.Context(), ".")
+		require.NoError(t, err)
+
+		modules, err := parser.EvaluateAll(t.Context())
+		require.NoError(t, err)
+
+		require.NotEmpty(t, instance.String())
+		require.Len(t, modules, 2)
+		instance.Reset()
+	})
+
+	//nolint:testifylint // linter wants `emptyf`, but the output is not legible
+	if !assert.Lenf(t, buf.Bytes(), 0, "logs detected in global logger, all logs should be using the instanced logger") {
+		t.Log(string(buf.Bytes())) // Helpful for debugging
+	}
+}