-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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.
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
Conversation
3d2d5d9
to
e64d935
Compare
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
assert.Equal(t, "test_value", attr.GetRawValue()) | ||
} | ||
|
||
func TestCyclicModules(t *testing.T) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
.
Hi @nikpivkin, thank you for working on this – I opened a PR against your branch which I hope will be helpful. |
fix: evaluateSteps after each submodule
lgtm, looks like we need to fix some merge conflicts @nikpivkin |
@simar7 Fixed |
Co-authored-by: William Reade <william@stacklet.io>
Description
Related issues
Checklist