From 13190e92d9fea1277389fc09fba0418c05c5f44f Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Thu, 4 Apr 2024 06:40:40 +0300 Subject: [PATCH] fix(terraform): eval submodules (#6411) Co-authored-by: William Reade --- .../scanners/terraform/parser/evaluator.go | 119 +++++++++++++----- .../scanners/terraform/parser/load_module.go | 8 ++ pkg/iac/scanners/terraform/parser/parser.go | 28 +++-- .../scanners/terraform/parser/parser_test.go | 109 ++++++++++++++++ 4 files changed, 223 insertions(+), 41 deletions(-) diff --git a/pkg/iac/scanners/terraform/parser/evaluator.go b/pkg/iac/scanners/terraform/parser/evaluator.go index 3633d22386a1..b93104f442cc 100644 --- a/pkg/iac/scanners/terraform/parser/evaluator.go +++ b/pkg/iac/scanners/terraform/parser/evaluator.go @@ -8,6 +8,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" @@ -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 @@ -126,48 +128,100 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str fsMap := make(map[string]fs.FS) fsMap[fsKey] = e.filesystem - var lastContext hcl.EvalContext e.debug.Log("Starting module evaluation...") - for i := 0; i < maxContextIterations; i++ { + e.evaluateSteps() - e.evaluateStep() + // expand out resources and modules via count, for-each and dynamic + // (not a typo, we do this twice so every order is processed) + e.blocks = e.expandBlocks(e.blocks) + e.blocks = e.expandBlocks(e.blocks) - // 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 - } + e.debug.Log("Starting submodule evaluation...") + submodules := e.loadSubmodules(ctx) - if len(e.ctx.Inner().Variables) != len(lastContext.Variables) { - lastContext.Variables = make(map[string]cty.Value, len(e.ctx.Inner().Variables)) + for i := 0; i < maxContextIterations; i++ { + changed := false + for _, sm := range submodules { + changed = changed || e.evaluateSubmodule(ctx, sm) } - for k, v := range e.ctx.Inner().Variables { - lastContext.Variables[k] = v + if !changed { + e.debug.Log("All submodules are evaluated at i=%d", i) + break } } - // expand out resources and modules via count, for-each and dynamic - // (not a typo, we do this twice so every order is processed) - e.blocks = e.expandBlocks(e.blocks) - e.blocks = e.expandBlocks(e.blocks) + e.debug.Log("Starting post-submodule evaluation...") + e.evaluateSteps() - e.debug.Log("Starting submodule evaluation...") 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("Module evaluation complete.") + rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) + return append(terraform.Modules{rootModule}, modules...), fsMap +} + +type submodule struct { + definition *ModuleDefinition + eval *evaluator + modules terraform.Modules + lastState map[string]cty.Value + fsMap map[string]fs.FS +} + +func (e *evaluator) loadSubmodules(ctx context.Context) []*submodule { + var submodules []*submodule + 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) + 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 } - // export module outputs - e.ctx.Set(outputs, "module", definition.Name) - modules = append(modules, submodules...) - for key, val := range definition.Parser.GetFilesystemMap() { - fsMap[key] = val + + 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 { + 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) + return false } } - e.debug.Log("Finished processing %d submodule(s).", len(modules)) - e.debug.Log("Starting post-submodule evaluation...") + e.debug.Log("Evaluating submodule %s", sm.definition.Name) + sm.eval.inputVars = inputVars + sm.modules, sm.fsMap = sm.eval.EvaluateAll(ctx) + outputs := sm.eval.exportOutputs() + + // lastState needs to be captured after applying outputs – so that they + // don't get treated as changes – but before running post-submodule + // evaluation, so that changes from that can trigger re-evaluations of + // the submodule if/when they feed back into inputs. + e.ctx.Set(outputs, "module", sm.definition.Name) + sm.lastState = sm.definition.inputVars() + e.evaluateSteps() + return true +} + +func (e *evaluator) evaluateSteps() { + var lastContext hcl.EvalContext for i := 0; i < maxContextIterations; i++ { e.evaluateStep() @@ -176,7 +230,6 @@ 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)) } @@ -184,10 +237,6 @@ func (e *evaluator) EvaluateAll(ctx context.Context) (terraform.Modules, map[str lastContext.Variables[k] = v } } - - e.debug.Log("Module evaluation complete.") - rootModule := terraform.NewModule(e.projectRootPath, e.modulePath, e.blocks, e.ignores) - return append(terraform.Modules{rootModule}, modules...), fsMap } func (e *evaluator) expandBlocks(blocks terraform.Blocks) terraform.Blocks { @@ -217,7 +266,9 @@ func (e *evaluator) expandDynamicBlock(b *terraform.Block) { b.InjectBlock(content, blockName) } } - sub.MarkExpanded() + if len(expanded) > 0 { + sub.MarkExpanded() + } } } @@ -246,6 +297,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 { diff --git a/pkg/iac/scanners/terraform/parser/load_module.go b/pkg/iac/scanners/terraform/parser/load_module.go index 461d7a7a1a56..0bd6a6395936 100644 --- a/pkg/iac/scanners/terraform/parser/load_module.go +++ b/pkg/iac/scanners/terraform/parser/load_module.go @@ -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 diff --git a/pkg/iac/scanners/terraform/parser/parser.go b/pkg/iac/scanners/terraform/parser/parser.go index b80c4a6babf2..b5b50dc913d7 100644 --- a/pkg/iac/scanners/terraform/parser/parser.go +++ b/pkg/iac/scanners/terraform/parser/parser.go @@ -2,6 +2,7 @@ package parser import ( "context" + "errors" "io" "io/fs" "os" @@ -224,18 +225,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)) @@ -246,7 +248,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)) } @@ -260,10 +262,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, @@ -278,11 +280,19 @@ func (p *Parser) EvaluateAll(ctx context.Context) (terraform.Modules, cty.Value, p.debug.Extend("evaluator"), p.allowDownloads, p.skipCachedModules, - ) - modules, fsMap := 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 := e.EvaluateAll(ctx) 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 { diff --git a/pkg/iac/scanners/terraform/parser/parser_test.go b/pkg/iac/scanners/terraform/parser/parser_test.go index 12594841251b..a20bb2a84b58 100644 --- a/pkg/iac/scanners/terraform/parser/parser_test.go +++ b/pkg/iac/scanners/terraform/parser/parser_test.go @@ -1522,3 +1522,112 @@ 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) { + files := map[string]string{ + "main.tf": ` +module "module2" { + source = "./modules/foo" + test_var = passthru.handover.from_1 +} + +// Demonstrates need for evaluateSteps between submodule evaluations. +resource "passthru" "handover" { + from_1 = module.module1.test_out + from_2 = module.module2.test_out +} + +module "module1" { + source = "./modules/bar" + test_var = passthru.handover.from_2 +} +`, + "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()) + } +}