diff --git a/pkg/iac/scanners/terraform/parser/evaluator.go b/pkg/iac/scanners/terraform/parser/evaluator.go index 9392664575..c2eaa07696 100644 --- a/pkg/iac/scanners/terraform/parser/evaluator.go +++ b/pkg/iac/scanners/terraform/parser/evaluator.go @@ -139,17 +139,19 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str e.blocks = e.expandBlocks(e.blocks) e.blocks = e.expandBlocks(e.blocks) - submodules := e.evaluateSubmodules(ctx, fsMap) + // rootModule is initialized here, but not fully evaluated until all submodules are evaluated. + // Initializing it up front to keep the module hierarchy of parents correct. + rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) + submodules := e.evaluateSubmodules(ctx, rootModule, 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 { +func (e *evaluator) evaluateSubmodules(ctx context.Context, parent *terraform.Module, fsMap map[string]fs.FS) terraform.Modules { submodules := e.loadSubmodules(ctx) if len(submodules) == 0 { @@ -174,6 +176,14 @@ func (e *evaluator) evaluateSubmodules(ctx context.Context, fsMap map[string]fs. var modules terraform.Modules for _, sm := range submodules { + // Assign the parent placeholder to any submodules without a parent. Any modules + // with a parent already have their correct parent placeholder assigned. + for _, submod := range sm.modules { + if submod.Parent() == nil { + submod.SetParent(parent) + } + } + modules = append(modules, sm.modules...) for k, v := range sm.fsMap { fsMap[k] = v diff --git a/pkg/iac/scanners/terraform/parser/parser_test.go b/pkg/iac/scanners/terraform/parser/parser_test.go index 08267c38dd..0e64d10144 100644 --- a/pkg/iac/scanners/terraform/parser/parser_test.go +++ b/pkg/iac/scanners/terraform/parser/parser_test.go @@ -7,6 +7,7 @@ import ( "log/slog" "os" "path/filepath" + "slices" "sort" "testing" "testing/fstest" @@ -18,6 +19,7 @@ import ( "github.com/aquasecurity/trivy/internal/testutil" "github.com/aquasecurity/trivy/pkg/iac/terraform" "github.com/aquasecurity/trivy/pkg/log" + "github.com/aquasecurity/trivy/pkg/set" ) func Test_BasicParsing(t *testing.T) { @@ -1714,6 +1716,20 @@ func TestNestedModulesOptions(t *testing.T) { var buf bytes.Buffer slog.SetDefault(slog.New(log.NewHandler(&buf, nil))) + // Folder structure + // ./ + // ├── main.tf + // └── modules + // ├── city + // │ └── main.tf + // ├── queens + // │ └── main.tf + // └── brooklyn + // └── main.tf + // + // Modules referenced + // main -> city ├─> brooklyn + // └─> queens files := map[string]string{ "main.tf": ` module "city" { @@ -1769,6 +1785,140 @@ module "invalid" { } require.NotContains(t, buf.String(), "failed to download") + + // Verify module parents are set correctly. + expectedParents := map[string]string{ + ".": "", + "modules/city": ".", + "modules/city/brooklyn": "modules/city", + "modules/city/queens": "modules/city", + } + + for _, mod := range modules { + expected, exists := expectedParents[mod.ModulePath()] + require.Truef(t, exists, "module %s does not exist in assertion", mod.ModulePath()) + if expected == "" { + require.Nil(t, mod.Parent()) + } else { + require.Equal(t, expected, mod.Parent().ModulePath(), "parent of module %q", mod.ModulePath()) + } + } +} + +// TestModuleParents sets up a nested module structure and verifies the +// parent-child relationships are correctly set. +func TestModuleParents(t *testing.T) { + // The setup is a list of continents, some countries, some cities, etc. + dirfs := os.DirFS("./testdata/nested") + parser := New(dirfs, "", + OptionStopOnHCLError(true), + OptionWithDownloads(false), + ) + require.NoError(t, parser.ParseFS(context.TODO(), ".")) + + modules, _, err := parser.EvaluateAll(context.TODO()) + require.NoError(t, err) + + // modules only have 'parent'. They do not have children, so create + // a structure that allows traversal from the root to the leafs. + modChildren := make(map[*terraform.Module][]*terraform.Module) + // Keep track of every module that exists + modSet := set.New[*terraform.Module]() + var root *terraform.Module + for _, mod := range modules { + mod := mod + modChildren[mod] = make([]*terraform.Module, 0) + modSet.Append(mod) + + if mod.Parent() == nil { + // Only 1 root should exist + require.Nil(t, root, "root module already set") + root = mod + } + modChildren[mod.Parent()] = append(modChildren[mod.Parent()], mod) + } + + type node struct { + prefix string + modulePath string + children []node + } + + // expectedTree is the full module tree structure. + expectedTree := node{ + modulePath: ".", + children: []node{ + { + modulePath: "north-america", + children: []node{ + { + modulePath: "north-america/united-states", + children: []node{ + {modulePath: "north-america/united-states/springfield", prefix: "illinois-"}, + {modulePath: "north-america/united-states/springfield", prefix: "idaho-"}, + {modulePath: "north-america/united-states/new-york", children: []node{ + {modulePath: "north-america/united-states/new-york/new-york-city"}, + }}, + }, + }, + { + modulePath: "north-america/canada", + children: []node{ + {modulePath: "north-america/canada/springfield", prefix: "ontario-"}, + }, + }, + }, + }, + }, + } + + var assertChild func(t *testing.T, n node, mod *terraform.Module) + assertChild = func(t *testing.T, n node, mod *terraform.Module) { + modSet.Remove(mod) + children := modChildren[mod] + + t.Run(n.modulePath, func(t *testing.T) { + if !assert.Equal(t, len(n.children), len(children), "modChildren count for %s", n.modulePath) { + return + } + for _, child := range children { + // Find the child module that we are expecting. + idx := slices.IndexFunc(n.children, func(node node) bool { + outputBlocks := child.GetBlocks().OfType("output") + outIdx := slices.IndexFunc(outputBlocks, func(outputBlock *terraform.Block) bool { + return outputBlock.Labels()[0] == "name" + }) + if outIdx == -1 { + return false + } + + output := outputBlocks[outIdx] + outVal := output.GetAttribute("value").Value() + if !outVal.Type().Equals(cty.String) { + return false + } + + modName := filepath.Base(node.modulePath) + if outVal.AsString() != node.prefix+modName { + return false + } + + return node.modulePath == child.ModulePath() + }) + if !assert.NotEqualf(t, -1, idx, "module prefix=%s path=%s not found in %s", n.prefix, child.ModulePath(), n.modulePath) { + continue + } + + assertChild(t, n.children[idx], child) + } + }) + + } + + assertChild(t, expectedTree, root) + // If any module was not asserted, the test will fail. This ensures the + // entire module tree is checked. + require.Equal(t, 0, modSet.Size(), "all modules asserted") } func TestCyclicModules(t *testing.T) { diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/main.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/main.tf new file mode 100644 index 0000000000..1012ab57bf --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/main.tf @@ -0,0 +1,9 @@ +module "north-america" { + source = "./north-america" +} + +output "all" { + value = [ + module.north-america, + ] +} \ No newline at end of file diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/canada/canada.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/canada/canada.tf new file mode 100644 index 0000000000..bb27996a01 --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/canada/canada.tf @@ -0,0 +1,17 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}canada" +} + +module "ontario-springfield" { + source = "./springfield" + prefix = "ontario-" +} + +output "ontario-springfield" { + value = module.ontario-springfield.name +} diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/canada/springfield/springfield.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/canada/springfield/springfield.tf new file mode 100644 index 0000000000..e8768e67e2 --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/canada/springfield/springfield.tf @@ -0,0 +1,8 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}springfield" +} \ No newline at end of file diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/north-america.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/north-america.tf new file mode 100644 index 0000000000..26b53f5f9d --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/north-america.tf @@ -0,0 +1,22 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}north-america" +} + +module "canada" { + source = "./canada" + prefix = "" +} + +module "united-states" { + source = "./united-states" + prefix = "" +} + +output "united-states" { + value = module.united-states.name +} \ No newline at end of file diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/new-york/new-york-city/new-york-city.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/new-york/new-york-city/new-york-city.tf new file mode 100644 index 0000000000..e0a0241797 --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/new-york/new-york-city/new-york-city.tf @@ -0,0 +1,8 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}new-york-city" +} \ No newline at end of file diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/new-york/new-york.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/new-york/new-york.tf new file mode 100644 index 0000000000..fc87248b57 --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/new-york/new-york.tf @@ -0,0 +1,17 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}new-york" +} + +module "new-york-city" { + source = "./new-york-city" + prefix = "" +} + +output "new-york-city" { + value = module.new-york-city.name +} diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/springfield/springfield.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/springfield/springfield.tf new file mode 100644 index 0000000000..e8768e67e2 --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/springfield/springfield.tf @@ -0,0 +1,8 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}springfield" +} \ No newline at end of file diff --git a/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/united-states.tf b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/united-states.tf new file mode 100644 index 0000000000..c6fff86f38 --- /dev/null +++ b/pkg/iac/scanners/terraform/parser/testdata/nested/north-america/united-states/united-states.tf @@ -0,0 +1,38 @@ +variable "prefix" { + type = string + default = "" +} + +output "name" { + value = "${var.prefix}united-states" +} + +// Same module twice, with different variables +module "illinois-springfield" { + source = "./springfield" + prefix = "illinois-" +} + +output "illinois-springfield" { + value = module.illinois-springfield.name +} + +module "idaho-springfield" { + source = "./springfield" + prefix = "idaho-" +} + +output "idaho-springfield" { + value = module.idaho-springfield.name +} + +module "new-york" { + source = "./new-york" + prefix = "" +} + +output "new-york" { + value = module.new-york.name +} + + diff --git a/pkg/iac/terraform/module.go b/pkg/iac/terraform/module.go index 09c406db15..35c9c1f579 100644 --- a/pkg/iac/terraform/module.go +++ b/pkg/iac/terraform/module.go @@ -47,6 +47,10 @@ func (c *Module) ModulePath() string { return c.modulePath } +func (c *Module) Parent() *Module { + return c.parent +} + func (c *Module) Ignores() ignore.Rules { return c.ignores }