Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(terraform): eval submodules #6411

Merged
merged 5 commits into from
Apr 4, 2024
Merged

fix(terraform): eval submodules #6411

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e64d935
fix(terraform): eval submodules
nikpivkin Mar 28, 2024
2e439e3
fix: evaluateSteps after each
fwereade Apr 1, 2024
6ad7e54
restore needed post-eval
fwereade Apr 2, 2024
58efb65
Merge pull request #5 from fwereade/tf-eval-submodules
nikpivkin Apr 2, 2024
51453e8
Merge branch 'main' into tf-eval-submodules
nikpivkin Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 83 additions & 35 deletions pkg/iac/scanners/terraform/parser/evaluator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/ext/typeexpr"
"github.com/samber/lo"
"github.com/zclconf/go-cty/cty"
"github.com/zclconf/go-cty/cty/convert"
"golang.org/x/exp/slices"
Expand Down Expand Up @@ -102,6 +103,7 @@ func (e *evaluator) evaluateStep() {

e.ctx.Set(e.getValuesByBlockType("data"), "data")
e.ctx.Set(e.getValuesByBlockType("output"), "output")
e.ctx.Set(e.getValuesByBlockType("module"), "module")
}

// exportOutputs is used to export module outputs to the parent module
Expand All @@ -128,25 +130,9 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str

var parseDuration time.Duration

var lastContext hcl.EvalContext
start := time.Now()
e.debug.Log("Starting module evaluation...")
for i := 0; i < maxContextIterations; 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) {
break
}

if len(e.ctx.Inner().Variables) != len(lastContext.Variables) {
lastContext.Variables = make(map[string]cty.Value, len(e.ctx.Inner().Variables))
}
for k, v := range e.ctx.Inner().Variables {
lastContext.Variables[k] = v
}
}
e.evaluateSteps()

// expand out resources and modules via count, for-each and dynamic
// (not a typo, we do this twice so every order is processed)
Expand All @@ -156,23 +142,85 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str
parseDuration += time.Since(start)

e.debug.Log("Starting submodule evaluation...")
var modules terraform.Modules
for _, definition := range e.loadModules(ctx) {
submodules, outputs, err := definition.Parser.EvaluateAll(ctx)
if err != nil {
e.debug.Log("Failed to evaluate submodule '%s': %s.", definition.Name, err)
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this didn't work as intended, is it because we weren't definition.Parser.Load(ctx) for each definition? Like so: https://github.com/aquasecurity/trivy/pull/6411/files#diff-b10704f6636c4e99c08df82aeb21c2283a75a61953d50b6f800289dbfa44979eR187

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modules are loaded and evaluated in alphabetical order. But modules can have different order and cyclic relationships, so we need to evaluate each module several times. Load is just a new function to load modules once, which returns an evaluator.

submodules := e.loadSubmodules(ctx)

for i := 0; i < maxContextIterations; i++ {
changed := false
for _, sm := range submodules {
changed = changed || e.evaluateSubmodule(ctx, sm)
}
// export module outputs
e.ctx.Set(outputs, "module", definition.Name)
modules = append(modules, submodules...)
for key, val := range definition.Parser.GetFilesystemMap() {
fsMap[key] = val
if !changed {
e.debug.Log("All submodules are evaluated at i=%d", i)
break
}
}

var modules terraform.Modules
for _, sm := range submodules {
modules = append(modules, sm.modules...)
fsMap = lo.Assign(fsMap, sm.fsMap)
}

e.debug.Log("Finished processing %d submodule(s).", len(modules))

e.debug.Log("Starting post-submodule evaluation...")
e.evaluateSteps()

e.debug.Log("Module evaluation complete.")
parseDuration += time.Since(start)
rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores)
return append(terraform.Modules{rootModule}, modules...), fsMap, parseDuration
}

type submodule struct {
definition *ModuleDefinition
eval *evaluator
modules terraform.Modules
lastState cty.Value
fsMap map[string]fs.FS
}

func (e *evaluator) loadSubmodules(ctx context.Context) []*submodule {
var submodules []*submodule

for _, definition := range e.loadModules(ctx) {
eval, err := definition.Parser.Load(ctx)
if errors.Is(err, ErrNoFiles) {
continue
} else if err != nil {
e.debug.Log("Failed to load submodule '%s': %s.", definition.Name, err)
continue
}

submodules = append(submodules, &submodule{
definition: definition,
eval: eval,
fsMap: make(map[string]fs.FS),
})
}

return submodules
}

func (e *evaluator) evaluateSubmodule(ctx context.Context, sm *submodule) bool {
sm.eval.inputVars = sm.definition.inputVars()
sm.modules, sm.fsMap, _ = sm.eval.EvaluateAll(ctx)
outputs := sm.eval.exportOutputs()

if reflect.DeepEqual(outputs, sm.lastState) {
e.debug.Log("Submodule %s outputs unchanged", sm.definition.Name)
return false
}
e.debug.Log("Submodule %s outputs changed", sm.definition.Name)

e.ctx.Set(outputs, "module", sm.definition.Name)
sm.lastState = outputs

return true
}

func (e *evaluator) evaluateSteps() {
var lastContext hcl.EvalContext
for i := 0; i < maxContextIterations; i++ {

e.evaluateStep()
Expand All @@ -181,19 +229,13 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str
if i > 0 && reflect.DeepEqual(lastContext.Variables, e.ctx.Inner().Variables) {
break
}

if len(e.ctx.Inner().Variables) != len(lastContext.Variables) {
lastContext.Variables = make(map[string]cty.Value, len(e.ctx.Inner().Variables))
}
for k, v := range e.ctx.Inner().Variables {
lastContext.Variables[k] = v
}
}

e.debug.Log("Module evaluation complete.")
parseDuration += time.Since(start)
rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores)
return append(terraform.Modules{rootModule}, modules...), fsMap, parseDuration
}

func (e *evaluator) expandBlocks(blocks terraform.Blocks) terraform.Blocks {
Expand Down Expand Up @@ -223,7 +265,9 @@ func (e *evaluator) expandDynamicBlock(b *terraform.Block) {
b.InjectBlock(content, blockName)
}
}
sub.MarkExpanded()
if len(expanded) > 0 {
sub.MarkExpanded()
}
}
}

Expand Down Expand Up @@ -252,6 +296,10 @@ func (e *evaluator) expandBlockForEaches(blocks terraform.Blocks, isDynamic bool
clones := make(map[string]cty.Value)
_ = forEachAttr.Each(func(key cty.Value, val cty.Value) {

if val.IsNull() {
return
}

// 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 {
Expand Down
8 changes: 8 additions & 0 deletions pkg/iac/scanners/terraform/parser/load_module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ type ModuleDefinition struct {
External bool
}

func (d *ModuleDefinition) inputVars() map[string]cty.Value {
inputs := d.Definition.Values().AsValueMap()
if inputs == nil {
return make(map[string]cty.Value)
}
return inputs
}

// loadModules reads all module blocks and loads them
func (e *evaluator) loadModules(ctx context.Context) []*ModuleDefinition {
var moduleDefinitions []*ModuleDefinition
Expand Down
28 changes: 19 additions & 9 deletions pkg/iac/scanners/terraform/parser/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package parser

import (
"context"
"errors"
"io"
"io/fs"
"os"
Expand Down Expand Up @@ -254,18 +255,19 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) error {
return nil
}

func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, error) {
var ErrNoFiles = errors.New("no files found")

func (p *Parser) Load(ctx context.Context) (*evaluator, error) {
p.debug.Log("Evaluating module...")

if len(p.files) == 0 {
p.debug.Log("No files found, nothing to do.")
return nil, cty.NilVal, nil
return nil, ErrNoFiles
}

blocks, ignores, err := p.readBlocks(p.files)
if err != nil {
return nil, cty.NilVal, err
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))

Expand All @@ -278,7 +280,7 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value,
} else {
inputVars, err = loadTFVars(p.configsFS, p.tfvarsPaths)
if err != nil {
return nil, cty.NilVal, err
return nil, err
}
p.debug.Log("Added %d variables from tfvars.", len(inputVars))
}
Expand All @@ -292,10 +294,10 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value,

workingDir, err := os.Getwd()
if err != nil {
return nil, cty.NilVal, err
return nil, err
}
p.debug.Log("Working directory for module evaluation is '%s'", workingDir)
evaluator := newEvaluator(
return newEvaluator(
p.moduleFS,
p,
p.projectRoot,
Expand All @@ -310,13 +312,21 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value,
p.debug.Extend("evaluator"),
p.allowDownloads,
p.skipCachedModules,
)
modules, fsMap, parseDuration := evaluator.EvaluateAll(ctx)
), nil
}

func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, error) {

e, err := p.Load(ctx)
if errors.Is(err, ErrNoFiles) {
return nil, cty.NilVal, nil
}
modules, fsMap, parseDuration := e.EvaluateAll(ctx)
p.metrics.Counts.Modules = len(modules)
p.metrics.Timings.ParseDuration = parseDuration
p.debug.Log("Finished parsing module '%s'.", p.moduleName)
p.fsMap = fsMap
return modules, evaluator.exportOutputs(), nil
return modules, e.exportOutputs(), nil
}

func (p *Parser) GetFilesystemMap() map[string]fs.FS {
Expand Down
103 changes: 103 additions & 0 deletions pkg/iac/scanners/terraform/parser/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1522,3 +1522,106 @@ func compareSets(a []int, b []int) bool {

return true
}

func TestModuleRefersToOutputOfAnotherModule(t *testing.T) {
files := map[string]string{
"main.tf": `
module "module2" {
source = "./modules/foo"
}

module "module1" {
source = "./modules/bar"
test_var = module.module2.test_out
}
`,
"modules/foo/main.tf": `
output "test_out" {
value = "test_value"
}
`,
"modules/bar/main.tf": `
variable "test_var" {}

resource "test_resource" "this" {
dynamic "dynamic_block" {
for_each = [var.test_var]
content {
some_attr = dynamic_block.value
}
}
}
`,
}

modules := parse(t, files)
require.Len(t, modules, 3)

resources := modules.GetResourcesByType("test_resource")
require.Len(t, resources, 1)

attr, _ := resources[0].GetNestedAttribute("dynamic_block.some_attr")
require.NotNil(t, attr)

assert.Equal(t, "test_value", attr.GetRawValue())
}

func TestCyclicModules(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nice test case! I'm curious what logic prevents the cyclic loop? In this case module1 uses module2 and vice-versa but how do we figure out to resolve them both?

does the logic still hold true if we have 3 modules in a cycle?

Copy link
Contributor Author

@nikpivkin nikpivkin Mar 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cycling between modules is allowed. Modules are evaluated until the output variables of all modules stop changing or the maximum number of evaluations is reached. In short, if at least one output variable of any module has been updated, we have one unevaluated module and therefore continue the evaluation.

Let's look at an example:

module "module1" {
	source = "./modules/too"
	test_var = module.module2.test_out
}

module "module2" {
	source = "./modules/bar"
	test_var = module.module3.test_out
}

module "module3" {
	source = "./modules/foo"
}

The first evaluation step will completely compute module3, the second module2, and the third module1.

files := map[string]string{
"main.tf": `
module "module2" {
source = "./modules/foo"
test_var = module.module1.test_out
}

module "module1" {
source = "./modules/bar"
test_var = module.module2.test_out
}
`,
"modules/foo/main.tf": `
variable "test_var" {}

resource "test_resource" "this" {
dynamic "dynamic_block" {
for_each = [var.test_var]
content {
some_attr = dynamic_block.value
}
}
}

output "test_out" {
value = "test_value"
}
`,
"modules/bar/main.tf": `
variable "test_var" {}

resource "test_resource" "this" {
dynamic "dynamic_block" {
for_each = [var.test_var]
content {
some_attr = dynamic_block.value
}
}
}

output "test_out" {
value = test_resource.this.dynamic_block.some_attr
}
`,
}

modules := parse(t, files)
require.Len(t, modules, 3)

resources := modules.GetResourcesByType("test_resource")
require.Len(t, resources, 2)

for _, res := range resources {
attr, _ := res.GetNestedAttribute("dynamic_block.some_attr")
require.NotNil(t, attr, res.FullName())
assert.Equal(t, "test_value", attr.GetRawValue())
}
}