From cc4f6bd986ee530b072391a7f7aac90fff8b2ae9 Mon Sep 17 00:00:00 2001 From: Andrea Cappelletti Date: Fri, 23 Oct 2026 14:26:07 -0400 Subject: [PATCH] fix(parser): evaluate module outputs (#116) Expressions reading module.. never resolved, so a count or for_each built from one dropped the block and an attribute set from one fell back to the catalog default. Module calls are now resolved before any resource is emitted: each call's instances are built, their own calls resolved recursively, and their output blocks evaluated in the module's scope. The outputs are added to the caller's scope as module. (a tuple under count, an object keyed by each.key under for_each), and the caller's locals are resolved again, before the next call. Calls are ordered so one that reads module.X, directly or through a local, comes after X. Resources are emitted afterwards from the resolved tree, in the same order as before. Output blocks in Terraform JSON files are translated (value only). --- CHANGELOG.md | 10 + internal/parser/terraform/json_config.go | 22 +- internal/parser/terraform/locals.go | 14 +- internal/parser/terraform/module_outputs.go | 163 +++++++++++ .../parser/terraform/module_outputs_test.go | 229 +++++++++++++++ internal/parser/terraform/modules.go | 273 ++++++++++++------ internal/parser/terraform/parser.go | 24 +- internal/parser/terraform/resources.go | 14 +- 8 files changed, 632 insertions(+), 117 deletions(-) create mode 100644 internal/parser/terraform/module_outputs.go create mode 100644 internal/parser/terraform/module_outputs_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 275e512..646db67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixed +- Module outputs are now evaluated (#116). An expression reading + `module..` never resolved: a `count` or `for_each` built + from one dropped the block, and an attribute set from one was priced at + the catalog default. Each module call is now resolved before the + resources that read it, its `output` blocks are evaluated in its own + scope, and the values are in scope for module inputs, locals and + resources, including `module.x[0].out` under `count`, + `module.x["key"].out` under `for_each`, and outputs passed up through + nested modules. Outputs in `.tf.json` files are read too. + - Homebrew: the cask now clears the quarantine flag after install. Homebrew quarantines what a cask downloads, and macOS killed the un-notarized binary on launch (exit 137), so `brew install c3xdev/tap/c3x` gave a c3x diff --git a/internal/parser/terraform/json_config.go b/internal/parser/terraform/json_config.go index 78cc5bb..c9b4df9 100644 --- a/internal/parser/terraform/json_config.go +++ b/internal/parser/terraform/json_config.go @@ -26,7 +26,8 @@ import ( // schemas. Translating keeps one AST type throughout and needs no schema: // // - The top-level structure is fixed by the language (resource, data, -// module, provider, variable, locals), so it becomes native blocks. +// module, provider, variable, locals, output), so it becomes native +// blocks. // - Inside a resource, a JSON object or array of objects is written as // an attribute: `root_block_device = { volume_size = 50 }`. That // evaluates to exactly the attribute shape a literal nested block @@ -41,9 +42,9 @@ import ( // references: a resource's `provider`, a module's `providers` map, // a variable's `type`, and a dynamic block's `iterator`. // -// Skipped, as irrelevant to cost: output, terraform, moved, import, check -// and removed blocks, and depends_on, lifecycle, provisioner and -// connection inside resources. Anything else this can't represent is +// Skipped, as irrelevant to cost: terraform, moved, import, check and +// removed blocks, everything in an output but its value, and depends_on, +// lifecycle, provisioner and connection inside resources. Anything else this can't represent is // skipped with a warning naming the file and key, never silently. // // Diagnostics for a JSON file cite positions in the translated source, @@ -144,7 +145,8 @@ const ( bodyResource bodyKind = iota // resource, data, and dynamic content bodyModule bodyVariable - bodyPlain // provider, locals: no special keys + bodyOutput // only value is kept + bodyPlain // provider, locals: no special keys ) func (t *jsonTranslator) skip(reason, key string) { @@ -157,7 +159,7 @@ func (t *jsonTranslator) file(obj *jsonObject) { for i, key := range obj.keys { val := obj.vals[i] switch key { - case "//", "output", "terraform", "moved", "import", "check", "removed": + case "//", "terraform", "moved", "import", "check", "removed": // Comments, and blocks that declare nothing c3x prices. case "resource", "data": t.typedBlocks(key, val) @@ -167,6 +169,8 @@ func (t *jsonTranslator) file(obj *jsonObject) { t.namedBlocks(key, val, bodyPlain) case "variable": t.namedBlocks(key, val, bodyVariable) + case "output": + t.namedBlocks(key, val, bodyOutput) case "locals": t.forEachObject(key, val, func(o *jsonObject) { t.b.WriteString("locals {\n") @@ -211,7 +215,7 @@ func (t *jsonTranslator) typedBlocks(blockType string, val any) { }) } -// namedBlocks writes module / provider / variable blocks: {"": body}, +// namedBlocks writes module / provider / variable / output blocks: {"": body}, // where body may be an array (several provider configurations). func (t *jsonTranslator) namedBlocks(blockType string, val any, kind bodyKind) { t.forEachObject(blockType, val, func(names *jsonObject) { @@ -283,6 +287,10 @@ func (t *jsonTranslator) attributes(obj *jsonObject, kind bodyKind, where string t.providersMap(val, addr) continue } + case bodyOutput: + if key != "value" { + continue + } case bodyVariable: switch key { case "type": diff --git a/internal/parser/terraform/locals.go b/internal/parser/terraform/locals.go index 840332b..eab4224 100644 --- a/internal/parser/terraform/locals.go +++ b/internal/parser/terraform/locals.go @@ -16,14 +16,19 @@ import ( // longest local→local reference chain. In practice that's < 5 and the // per-attribute work is cheap, so we don't bother with a smarter // dependency-graph topological sort. -func resolveLocals(scope evalScope, sources []sourceFile, vars map[string]cty.Value, data cty.Value, logger *slog.Logger) map[string]cty.Value { +// +// modules is the module. scope: the outputs of the module calls +// resolved so far. Locals are resolved again as each call is resolved, so +// a nil logger skips the warnings for those intermediate passes. +func resolveLocals(scope evalScope, sources []sourceFile, vars map[string]cty.Value, data, modules cty.Value, logger *slog.Logger) map[string]cty.Value { + extras := map[string]cty.Value{"module": modules} pending := collectLocalAttributes(sources) resolved := map[string]cty.Value{} for { progressed := false var still []localAttr for _, la := range pending { - ctx := scope.evalContext(asObject(vars), asObject(resolved), data, nil) + ctx := scope.evalContext(asObject(vars), asObject(resolved), data, extras) val, diags := la.Expr.Value(ctx) if diags.HasErrors() { still = append(still, la) @@ -37,10 +42,13 @@ func resolveLocals(scope evalScope, sources []sourceFile, vars map[string]cty.Va break } } + if logger == nil { + return resolved + } // Whatever never resolved: report it if the cause is a function we // don't implement, since every resource reading that local is affected. for _, la := range pending { - _, diags := la.Expr.Value(scope.evalContext(asObject(vars), asObject(resolved), data, nil)) + _, diags := la.Expr.Value(scope.evalContext(asObject(vars), asObject(resolved), data, extras)) warnUnknownFunctions(diags, logger, "local."+la.Name) } return resolved diff --git a/internal/parser/terraform/module_outputs.go b/internal/parser/terraform/module_outputs.go new file mode 100644 index 0000000..ddf7e74 --- /dev/null +++ b/internal/parser/terraform/module_outputs.go @@ -0,0 +1,163 @@ +package terraform + +import ( + "github.com/hashicorp/hcl/v2" + "github.com/zclconf/go-cty/cty" +) + +// moduleOutputs evaluates a module instance's `output "x" { value = ... }` +// blocks in its own scope. An output that can't be evaluated is left out, +// so reading it fails in the caller exactly as an unknown attribute does. +func moduleOutputs(env moduleEnv, sources []sourceFile) cty.Value { + ctx := env.evalContext(nil) + out := map[string]cty.Value{} + for _, src := range sources { + for _, block := range src.Body.Blocks { + if block.Type != "output" || len(block.Labels) == 0 { + continue + } + attr, ok := block.Body.Attributes["value"] + if !ok { + continue + } + val, diags := attr.Expr.Value(ctx) + if diags.HasErrors() { + env.logger.Debug("module output evaluation failed", + "output", block.Labels[0], "dir", env.scope.moduleDir, + "diags", formatDiags(diags)) + continue + } + out[block.Labels[0]] = val + } + } + return cty.ObjectVal(out) +} + +// callOrder orders a configuration's module calls so that a call reading +// module.X, directly or through a local, comes after the call X. Calls +// that depend on nothing keep their source order. A cycle, which +// Terraform rejects unless it runs through outputs that don't depend on +// each other, is broken at the first remaining call in source order. +// +// localsReadModules reports whether any local reads a module output, in +// which case the locals are resolved again after each call. +func callOrder(calls []*moduleCall, sources []sourceFile) (order []int, localsReadModules bool) { + localDeps := localModuleDeps(sources) + for _, d := range localDeps { + if len(d) > 0 { + localsReadModules = true + break + } + } + names := map[string]bool{} + deps := make([]map[string]bool, len(calls)) + for i, c := range calls { + names[c.Labels[0]] = true + deps[i] = map[string]bool{} + for _, attr := range c.Body.Attributes { + addModuleDeps(deps[i], attr.Expr.Variables(), localDeps) + } + delete(deps[i], c.Labels[0]) + } + done := map[string]bool{} + placed := make([]bool, len(calls)) + for len(order) < len(calls) { + next := -1 + for i := range calls { + if placed[i] { + continue + } + if next < 0 { + next = i + } + if ready(deps[i], names, done) { + next = i + break + } + } + placed[next] = true + order = append(order, next) + done[calls[next].Labels[0]] = true + } + return order, localsReadModules +} + +// ready reports whether every module call in deps has been resolved; +// names the configuration doesn't call can never be, so they don't wait. +func ready(deps, names, done map[string]bool) bool { + for d := range deps { + if names[d] && !done[d] { + return false + } + } + return true +} + +// localModuleDeps maps each local to the module calls it reads, directly +// or through other locals. +func localModuleDeps(sources []sourceFile) map[string]map[string]bool { + attrs := collectLocalAttributes(sources) + direct := map[string]map[string]bool{} + refs := map[string][]string{} + for _, la := range attrs { + direct[la.Name] = map[string]bool{} + for _, t := range la.Expr.Variables() { + switch t.RootName() { + case "module": + if name, ok := secondStep(t); ok { + direct[la.Name][name] = true + } + case "local": + if name, ok := secondStep(t); ok { + refs[la.Name] = append(refs[la.Name], name) + } + } + } + } + // Close over local → local references; chains are short. + for changed := true; changed; { + changed = false + for name, deps := range direct { + for _, r := range refs[name] { + for d := range direct[r] { + if !deps[d] { + deps[d] = true + changed = true + } + } + } + } + } + return direct +} + +// addModuleDeps adds the module calls a set of traversals reads, directly +// or through locals, to deps. +func addModuleDeps(deps map[string]bool, traversals []hcl.Traversal, localDeps map[string]map[string]bool) { + for _, t := range traversals { + name, ok := secondStep(t) + if !ok { + continue + } + switch t.RootName() { + case "module": + deps[name] = true + case "local": + for d := range localDeps[name] { + deps[d] = true + } + } + } +} + +// secondStep returns the attribute name after a traversal's root: +// "sizes" for module.sizes.x. +func secondStep(t hcl.Traversal) (string, bool) { + if len(t) < 2 { + return "", false + } + if a, ok := t[1].(hcl.TraverseAttr); ok { + return a.Name, true + } + return "", false +} diff --git a/internal/parser/terraform/module_outputs_test.go b/internal/parser/terraform/module_outputs_test.go new file mode 100644 index 0000000..b68dc54 --- /dev/null +++ b/internal/parser/terraform/module_outputs_test.go @@ -0,0 +1,229 @@ +package terraform_test + +import ( + "os" + "path/filepath" + "slices" + "testing" + + "github.com/c3xdev/c3x/internal/parser/terraform" +) + +// writeTree writes files (relative path → body) under dir. +func writeTree(t *testing.T, dir string, files map[string]string) { + t.Helper() + for rel, body := range files { + p := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + write(t, filepath.Dir(p), filepath.Base(p), body) + } +} + +func parseOffline(t *testing.T, dir string) map[string]map[string]any { + t.Helper() + got, err := terraform.ParseDirectory(dir, terraform.Options{Offline: true}) + if err != nil { + t.Fatal(err) + } + out := map[string]map[string]any{} + for name, r := range byName(got) { + out[name] = r.Attributes + } + return out +} + +// The reproduction from c3x#116: a module's outputs feed another module's +// inputs (a for_each and an attribute) and a root resource's attribute. +// The queues used to be dropped and both instance classes priced at the +// catalog default. +func TestModuleOutputsFeedInputsAndResources(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "mods/sizes/main.tf": ` + locals { + sizes = { prod = { class = "db.m7g.large" } } + } + output "sizes" { value = local.sizes } + output "queues" { value = ["a", "b"] } + `, + "mods/db/main.tf": ` + variable "class" { type = string } + variable "queues" { type = list(string) } + resource "aws_db_instance" "x" { + engine = "postgres" + instance_class = var.class + allocated_storage = 50 + } + resource "aws_sqs_queue" "q" { + for_each = toset(var.queues) + name = each.key + } + `, + "root/main.tf": ` + provider "aws" { region = "eu-west-1" } + module "sizes" { source = "../mods/sizes" } + module "db" { + source = "../mods/db" + class = module.sizes.sizes["prod"].class + queues = module.sizes.queues + } + resource "aws_db_instance" "direct" { + engine = "postgres" + instance_class = module.sizes.sizes["prod"].class + allocated_storage = 50 + } + `, + }) + rs := parseOffline(t, filepath.Join(dir, "root")) + for _, name := range []string{"direct", "module.db.x"} { + if got := rs[name]["instance_class"]; got != "db.m7g.large" { + t.Errorf("%s instance_class = %v, want db.m7g.large", name, got) + } + } + for _, name := range []string{`module.db.q["a"]`, `module.db.q["b"]`} { + if _, ok := rs[name]; !ok { + t.Errorf("%s missing; got %v", name, keys(rs)) + } + } +} + +// A call is resolved after the modules it reads even when it is written +// first, and a local built from a module output is in scope for both +// module inputs and resources. +func TestModuleOutputsThroughLocalsInAnyOrder(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "main.tf": ` + module "web" { + source = "./web" + type = local.type + } + locals { type = module.cells.types.web } + resource "aws_instance" "bastion" { + instance_type = local.type + } + module "cells" { source = "./cells" } + `, + "cells/main.tf": ` + variable "web" { default = "m5.large" } + output "types" { value = { web = var.web } } + `, + "web/main.tf": ` + variable "type" {} + resource "aws_instance" "this" { instance_type = var.type } + `, + }) + rs := parseOffline(t, dir) + for _, name := range []string{"bastion", "module.web.this"} { + if got := rs[name]["instance_type"]; got != "m5.large" { + t.Errorf("%s instance_type = %v, want m5.large", name, got) + } + } +} + +// Outputs of counted and for_each modules are addressed as Terraform +// addresses them: module.x[0].out and module.x["key"].out. Outputs of a +// nested module reach the root through the module in between. +func TestModuleOutputsOfInstancesAndNestedModules(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "main.tf": ` + module "counted" { + source = "./size" + count = 2 + size = count.index == 0 ? "t3.small" : "t3.large" + } + module "keyed" { + source = "./size" + for_each = toset(["x"]) + size = "m5.${each.key}large" + } + module "outer" { source = "./outer" } + resource "aws_instance" "a" { instance_type = module.counted[1].size } + resource "aws_instance" "b" { instance_type = module.keyed["x"].size } + resource "aws_instance" "c" { instance_type = module.outer.size } + `, + "size/main.tf": ` + variable "size" {} + output "size" { value = var.size } + `, + "outer/main.tf": ` + module "inner" { + source = "../size" + size = "c5.large" + } + output "size" { value = module.inner.size } + `, + }) + rs := parseOffline(t, dir) + for name, want := range map[string]string{"a": "t3.large", "b": "m5.xlarge", "c": "c5.large"} { + if got := rs[name]["instance_type"]; got != want { + t.Errorf("%s instance_type = %v, want %s", name, got, want) + } + } +} + +// A count from a module output expands the resource, and outputs work +// from Terraform JSON modules too. +func TestModuleOutputsFromJSONDriveCount(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "main.tf": ` + module "cfg" { source = "./cfg" } + resource "aws_instance" "n" { + count = module.cfg.nodes + instance_type = "t3.micro" + } + `, + "cfg/main.tf.json": `{ + "output": { "nodes": { "value": 3, "description": "node count", "sensitive": false } } + }`, + }) + rs := parseOffline(t, dir) + if got := keys(rs); !slices.Equal(got, []string{"n[0]", "n[1]", "n[2]"}) { + t.Fatalf("got %v, want three instances of n", got) + } +} + +// Two modules reading each other's outputs can't both resolve; the parse +// finishes, and what doesn't resolve is reported as before. +func TestModuleOutputCycleTerminates(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "main.tf": ` + module "a" { + source = "./echo" + in = module.b.out + } + module "b" { + source = "./echo" + in = module.a.out + } + `, + "echo/main.tf": ` + variable "in" { default = "t3.micro" } + output "out" { value = var.in } + resource "aws_instance" "this" { instance_type = var.in } + `, + }) + rs := parseOffline(t, dir) + if len(rs) != 2 { + t.Fatalf("got %v, want module.a.this and module.b.this", keys(rs)) + } +} + +func keys(m map[string]map[string]any) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + slices.Sort(out) + return out +} diff --git a/internal/parser/terraform/modules.go b/internal/parser/terraform/modules.go index 5603e06..123e5c4 100644 --- a/internal/parser/terraform/modules.go +++ b/internal/parser/terraform/modules.go @@ -52,20 +52,47 @@ func loadInitModules(baseDir string, logger *slog.Logger) map[string]string { return out } -// moduleExpander carries what stays fixed while expanding a module tree. +// moduleExpander carries what stays fixed while resolving a module tree. type moduleExpander struct { initModules map[string]string offline bool logger *slog.Logger - out *[]domain.Resource } -// expandModules walks every `module "X" { ... }` block and recursively -// parses each module's source directory, threading parent inputs in as -// the child's variables and prefixing emitted Resources with the -// `module.X.` path. +// moduleNode is one module instance: the scope its expressions evaluate +// in, its configuration, the address prefix of its resources (empty for +// the root, `module.x[0].` below it), and the module instances it calls, +// in source order. +type moduleNode struct { + env moduleEnv + sources []sourceFile + prefix string + children []*moduleNode +} + +// emit appends the resources of n and of every module instance under it. +func (n *moduleNode) emit(out *[]domain.Resource) error { + if err := emitResources(n.env, n.sources, n.prefix, out); err != nil { + return err + } + for _, c := range n.children { + if err := c.emit(out); err != nil { + return err + } + } + return nil +} + +// resolveCalls resolves every `module "X" { ... }` block of n: it parses +// each module's source directory, threading n's values in as the child's +// variables, recurses into the child's own module calls, and evaluates +// the child's `output` blocks. Each call's outputs are added to n's scope +// as module.X, and n's locals are resolved again, before the next call is +// resolved, so a module input, a local or a resource of n can read +// another module's outputs. Calls are resolved in dependency order (see +// callOrder); n's locals are resolved a final time once all are done. // -// A module block with count or for_each is expanded once per instance, +// A module block with count or for_each is resolved once per instance, // with count.index / each.key / each.value in scope for its inputs, and // its resources are addressed module.X[0]. / module.X["a"]., as Terraform // addresses them. Every instance counts against the parse budget. @@ -74,95 +101,152 @@ type moduleExpander struct { // the manifest produced by `terraform init`. Sources we can't resolve // (no manifest entry) log a loud warning and are skipped — the rest of // the config still produces a useful estimate. -func (m moduleExpander) expandModules( - env moduleEnv, - baseDir string, - sources []sourceFile, - namePrefix string, - keyPrefix string, - depth int, -) error { - logger := m.logger - if depth >= MaxModuleDepth { - logger.Warn("module nesting exceeded MaxModuleDepth; truncating expansion. "+ +func (m moduleExpander) resolveCalls(n *moduleNode, baseDir, keyPrefix string, depth int) error { + calls := moduleBlocks(n.sources) + if len(calls) > 0 && depth >= MaxModuleDepth { + m.logger.Warn("module nesting exceeded MaxModuleDepth; truncating expansion. "+ "likely a self-referential `module \"x\" { source = \".\" }`", "depth", depth, "max", MaxModuleDepth) - return nil + calls = nil } - parentCtx := env.evalContext(nil) + order, localsReadModules := callOrder(calls, n.sources) + byCall := make([][]*moduleNode, len(calls)) + for _, i := range order { + nodes, outputs, ok, err := m.resolveCall(n, calls[i], baseDir, keyPrefix, depth) + if err != nil { + return err + } + if !ok { + continue + } + byCall[i] = nodes + if n.env.modules == nil { + n.env.modules = map[string]cty.Value{} + } + n.env.modules[calls[i].Labels[0]] = outputs + if localsReadModules { + n.env.locals = resolveLocals(n.env.scope, n.sources, n.env.vars, n.env.data, asObject(n.env.modules), nil) + } + } + n.env.locals = resolveLocals(n.env.scope, n.sources, n.env.vars, n.env.data, asObject(n.env.modules), m.logger) + for _, nodes := range byCall { + n.children = append(n.children, nodes...) + } + return nil +} +// moduleBlocks lists the module calls of a configuration. +func moduleBlocks(sources []sourceFile) []*moduleCall { + var out []*moduleCall for _, src := range sources { for _, block := range src.Body.Blocks { - if block.Type != "module" || len(block.Labels) == 0 { - continue + if block.Type == "module" && len(block.Labels) > 0 { + out = append(out, &moduleCall{Block: block, path: src.Path}) } - modName := block.Labels[0] - address := namePrefix + "module." + modName + } + } + return out +} - instances, ok := expandInstances(env, block.Body, src.Path, address) - if !ok { - continue - } - if err := env.scope.budget.moduleCall(address, len(instances)); err != nil { - return err - } - if len(instances) == 0 { - continue - } +// moduleCall is a module block and the file it is in. +type moduleCall struct { + *hclsyntax.Block + path string +} - source, ok := readModuleSource(block.Body, parentCtx) - if !ok { - continue - } - version := readModuleVersion(block.Body, parentCtx) +// resolveCall resolves one module call into its instances, and returns +// the value module. takes in the caller: the outputs object, a +// tuple of them under count, or an object keyed by each.key under +// for_each. ok is false when the call is skipped. +func (m moduleExpander) resolveCall( + parent *moduleNode, + block *moduleCall, + baseDir, keyPrefix string, + depth int, +) (nodes []*moduleNode, outputs cty.Value, ok bool, err error) { + logger := m.logger + env := parent.env + modName := block.Labels[0] + address := parent.prefix + "module." + modName - // The init manifest keys modules by name, never by instance. - manifestKey := modName - if keyPrefix != "" { - manifestKey = keyPrefix + "." + modName - } + instances, ok := expandInstances(env, block.Body, block.path, address) + if !ok { + return nil, cty.NilVal, false, nil + } + if err := env.scope.budget.moduleCall(address, len(instances)); err != nil { + return nil, cty.NilVal, false, err + } + _, counted := block.Body.Attributes["count"] + _, keyed := block.Body.Attributes["for_each"] + if len(instances) == 0 { + if counted { + return nil, cty.EmptyTupleVal, true, nil + } + return nil, cty.EmptyObjectVal, true, nil + } - childDir, ok := resolveModuleSource(source, version, baseDir, manifestKey, m.initModules, modName, m.offline, logger) - if !ok { - continue - } - if info, err := os.Stat(childDir); err != nil || !info.IsDir() { - logger.Warn("module source path is not a directory; skipping", - "module", modName, "dir", childDir) - continue - } - // Untrusted input may not pull configuration from outside its own - // tree: on a server, source = "../other-upload" (or a committed - // .terraform/modules/modules.json pointing there) would fold - // another user's infrastructure into this estimate. Terraform - // allows ../ sources, so trusted parses keep doing the same. - if env.scope.untrusted && !env.scope.containsDir(childDir) { - logger.Warn("untrusted input: module source is outside the project directory; skipping", - "module", modName, "dir", childDir) - continue - } + parentCtx := env.evalContext(nil) + source, ok := readModuleSource(block.Body, parentCtx) + if !ok { + return nil, cty.NilVal, false, nil + } + version := readModuleVersion(block.Body, parentCtx) - childSources, err := loadConfigDir(childDir, logger) - if err != nil { - return fmt.Errorf("module %s: %w", modName, err) - } - child := moduleSource{ - dir: childDir, - sources: childSources, - types: collectVariableTypes(childSources), - resources: collectLiteralResources(childSources), - } - for _, inst := range instances { - if err := m.expandInstance( - env, block, child, inst, - address+inst.suffix+".", manifestKey, depth, - ); err != nil { - return err - } - } + // The init manifest keys modules by name, never by instance. + manifestKey := modName + if keyPrefix != "" { + manifestKey = keyPrefix + "." + modName + } + + childDir, ok := resolveModuleSource(source, version, baseDir, manifestKey, m.initModules, modName, m.offline, logger) + if !ok { + return nil, cty.NilVal, false, nil + } + if info, err := os.Stat(childDir); err != nil || !info.IsDir() { + logger.Warn("module source path is not a directory; skipping", + "module", modName, "dir", childDir) + return nil, cty.NilVal, false, nil + } + // Untrusted input may not pull configuration from outside its own + // tree: on a server, source = "../other-upload" (or a committed + // .terraform/modules/modules.json pointing there) would fold + // another user's infrastructure into this estimate. Terraform + // allows ../ sources, so trusted parses keep doing the same. + if env.scope.untrusted && !env.scope.containsDir(childDir) { + logger.Warn("untrusted input: module source is outside the project directory; skipping", + "module", modName, "dir", childDir) + return nil, cty.NilVal, false, nil + } + + childSources, err := loadConfigDir(childDir, logger) + if err != nil { + return nil, cty.NilVal, false, fmt.Errorf("module %s: %w", modName, err) + } + child := moduleSource{ + dir: childDir, + sources: childSources, + types: collectVariableTypes(childSources), + resources: collectLiteralResources(childSources), + } + var tuple []cty.Value + byKey := map[string]cty.Value{} + for _, inst := range instances { + node, err := m.resolveInstance(env, block.Block, child, inst, address+inst.suffix+".", manifestKey, depth) + if err != nil { + return nil, cty.NilVal, false, err } + nodes = append(nodes, node) + out := moduleOutputs(node.env, child.sources) + tuple = append(tuple, out) + byKey[inst.key] = out } - return nil + switch { + case counted: + return nodes, cty.TupleVal(tuple), true, nil + case keyed: + return nodes, cty.ObjectVal(byKey), true, nil + } + return nodes, tuple[0], true, nil } // moduleSource is a module's configuration, loaded once per module block @@ -174,18 +258,18 @@ type moduleSource struct { resources map[string]cty.Value } -// expandInstance parses one instance of a module call: its inputs are -// evaluated in the parent's scope plus the instance's count / each, -// then the child's resources are emitted under childPrefix and its own -// module calls recursed into. -func (m moduleExpander) expandInstance( +// resolveInstance resolves one instance of a module call: its inputs are +// evaluated in the parent's scope plus the instance's count / each, then +// the child's own module calls are resolved in turn. Its resources are +// emitted later, under childPrefix (see moduleNode.emit). +func (m moduleExpander) resolveInstance( parent moduleEnv, block *hclsyntax.Block, child moduleSource, inst instance, childPrefix, manifestKey string, depth int, -) error { +) (*moduleNode, error) { logger := m.logger modName := block.Labels[0] instCtx := parent.evalContext(inst.extras) @@ -210,10 +294,10 @@ func (m moduleExpander) expandInstance( childVars, err := collectVariableDefaults(child.sources) if err != nil { - return err + return nil, err } if err := applyAutoTfvars(child.dir, childVars); err != nil { - return err + return nil, err } for k, v := range childInputs { childVars[k] = v @@ -235,7 +319,7 @@ func (m moduleExpander) expandInstance( ) childData := collectDataBlocks(child.sources, regionHints) childScope := parent.scope.child(child.dir) - childLocals := resolveLocals(childScope, child.sources, childVars, childData, logger) + childLocals := resolveLocals(childScope, child.sources, childVars, childData, cty.EmptyObjectVal, nil) if r := findDefaultRegion(child.sources, childVars, childLocals, childData, logger); r != "" { childFallback = r } @@ -256,10 +340,11 @@ func (m moduleExpander) expandInstance( // count / for_each makes every resource under it a guess too. assumedCount: joinAssumed(parent.assumedCount, inst.assumed), } - if err := emitResources(env, child.sources, childPrefix, m.out); err != nil { - return err + node := &moduleNode{env: env, sources: child.sources, prefix: childPrefix} + if err := m.resolveCalls(node, child.dir, manifestKey, depth+1); err != nil { + return nil, err } - return m.expandModules(env, child.dir, child.sources, childPrefix, manifestKey, depth+1) + return node, nil } // readModuleSource evaluates a module block's `source = "..."` attribute diff --git a/internal/parser/terraform/parser.go b/internal/parser/terraform/parser.go index cb13df2..e3f576d 100644 --- a/internal/parser/terraform/parser.go +++ b/internal/parser/terraform/parser.go @@ -199,8 +199,10 @@ func parseSources(sources []sourceFile, baseDir string, opts Options) ([]domain. regionHints := collectProviderRegions(sources, variables, nil, cty.EmptyObjectVal, "", logger) data := collectDataBlocks(sources, regionHints) - // Stage 4: locals — fixed-point against (var, data). - locals := resolveLocals(scope, sources, variables, data, logger) + // Stage 4: locals — fixed-point against (var, data). Resolved quietly: + // stage 7 resolves them again once module outputs are known, and + // reports what still doesn't resolve then. + locals := resolveLocals(scope, sources, variables, data, cty.EmptyObjectVal, nil) // Stage 5: provider regions (after vars/locals/data are populated so // `provider "aws" { region = var.region }` resolves). Each resource is @@ -219,19 +221,21 @@ func parseSources(sources []sourceFile, baseDir string, opts Options) ([]domain. logger: logger, } - // Stage 6: walk resource blocks; each contributes one or more - // domain.Resource entries depending on count / for_each. - var resources []domain.Resource - if err := emitResources(env, sources, "", &resources); err != nil { + // Stage 6: resolve module calls recursively, so module outputs are in + // scope (module..) before any resource is evaluated. + root := &moduleNode{env: env, sources: sources} + expander := moduleExpander{initModules: initModules, offline: opts.Offline, logger: logger} + if err := expander.resolveCalls(root, baseDir, "", 0); err != nil { return nil, err } - // Stage 7: recursively expand modules. - expander := moduleExpander{initModules: initModules, offline: opts.Offline, logger: logger, out: &resources} - if err := expander.expandModules(env, baseDir, sources, "", "", 0); err != nil { + // Stage 7: walk resource blocks, root first and then each module + // instance; each block contributes one or more domain.Resource + // entries depending on count / for_each. + var resources []domain.Resource + if err := root.emit(&resources); err != nil { return nil, err } - return resources, nil } diff --git a/internal/parser/terraform/resources.go b/internal/parser/terraform/resources.go index 7f6ec09..e3c4458 100644 --- a/internal/parser/terraform/resources.go +++ b/internal/parser/terraform/resources.go @@ -12,7 +12,8 @@ import ( // moduleEnv is everything the expressions of one module instance are // evaluated against: its variables, locals, data placeholders, the -// literal attributes of its resources, and its provider regions. +// literal attributes of its resources, the outputs of the modules it +// calls, and its provider regions. type moduleEnv struct { scope evalScope vars map[string]cty.Value @@ -21,8 +22,11 @@ type moduleEnv struct { // resources exposes the statically known attributes of the module's // resources as ..; see collectLiteralResources. resources map[string]cty.Value - regions providerRegions - logger *slog.Logger + // modules holds the outputs of the module calls resolved so far, by + // call name, exposed as module..; see resolveCalls. + modules map[string]cty.Value + regions providerRegions + logger *slog.Logger // assumedCount is set when this module instance exists because of a // module-level count / for_each computed from a data source // placeholder; every resource under it inherits it (see @@ -34,6 +38,7 @@ type moduleEnv struct { // per-instance scopes (count, each). func (e moduleEnv) evalContext(extras map[string]cty.Value) *hcl.EvalContext { ctx := e.scope.evalContext(asObject(e.vars), asObject(e.locals), e.data, extras) + ctx.Variables["module"] = asObject(e.modules) for k, v := range e.resources { if _, taken := ctx.Variables[k]; !taken { ctx.Variables[k] = v @@ -100,6 +105,8 @@ func emitOne( // suffix ("", "[0]", `["a"]`) and the count / each scope it evaluates in. type instance struct { suffix string + // key is the for_each key, which addresses the instance's outputs. + key string extras map[string]cty.Value // assumed describes the data source placeholders the count / // for_each was computed from; empty when it rests on none. @@ -171,6 +178,7 @@ func expandInstances(env moduleEnv, body *hclsyntax.Body, srcPath, address strin // from each.key / each.value are reported unresolved. out[i] = instance{ suffix: fmt.Sprintf("[%q]", p.Key), + key: p.Key, assumed: assumed, extras: map[string]cty.Value{ "each": cty.ObjectVal(map[string]cty.Value{