Fix infinite loop on variable resolution

[dinedal]

When executing on a terraform HCL IaC, it is possible for a variable to end up resolving to itself because it is undefined at the time of analysis (defined in tfvars or on the command line)

This logic detects when a variable resolves to itself and breaks out of the infinite loop that would be created.

fixes #406

[codecov]

Codecov Report

Merging #393 (d356d5c) into master (9b32df3) will decrease coverage by 0.19%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
- Coverage   63.55%   63.35%   -0.20%     
==========================================
  Files          85       85              
  Lines        1904     1910       +6     
==========================================
  Hits         1210     1210              
- Misses        591      595       +4     
- Partials      103      105       +2     

Impacted Files	Coverage Δ
...iac-providers/terraform/v12/variable-references.go	58.33% <0.00%> (-6.49%)	⬇️

[kanchwala-yusuf]

Thank you for your contribution.

This fix breaks the infinite loop, but does not completely fix the problem of resolving variables initialized in the Parent module.

We may need to think of a way to resolve variables referenced in the Parent Module call as well.

[dinedal]

@kanchwala-yusuf maybe you could explain a little more on this?

We may need to think of a way to resolve variables the variables referenced in the Parent Module call as well.

The function I am patching is called ResolveVarRefFromParentModuleCall - which place are you talking about where we need to "resolve variables the variables referenced in the Parent Module call" that isn't here? What do you mean by "resolve variables the variables referenced" ?

[dinedal]

@kanchwala-yusuf - I applied the same fix in ResolveVarRef() at c89bb6f - maybe this is what you mean?

[kanchwala-yusuf]

@dinedal we do not require the change here

[kanchwala-yusuf]

Come to think of it, this may not hurt us at all.

[kanchwala-yusuf]

@kanchwala-yusuf maybe you could explain a little more on this?

We may need to think of a way to resolve variables the variables referenced in the Parent Module call as well.

The function I am patching is called ResolveVarRefFromParentModuleCall - which place are you talking about where we need to "resolve variables the variables referenced in the Parent Module call" that isn't here? What do you mean by "resolve variables the variables referenced" ?

Sorry for the typo there. I meant "We may need to think of a way to resolve variables referenced in the Parent Module call as well."

I have also edited my previous comment.

[kanchwala-yusuf]

Hey @dinedal,

Let me elaborate more on how we resolve variables.

Variable values initialized in the parent module call take precedence over values initialized in the same module, hence we execute the following steps:

• first try to resolve variables initialized in parent module call
• if we fail in the first step then try to resolve variables initialized in same module

Now, issue is the parent module call itself references to another variable. That's why terrascan is stuck in the infinite loop.

Your fix breaks the loop but, does not resolve variables in the parent module call.

We need to do figure out a way to do that.

[dinedal]

@kanchwala-yusuf OK, I don't understand, let me tell you my understanding and maybe you can help point out what I'm not seeing.

1. references.go:ResolveStrRef(ref string) is called as the entry point into resolution of a variable.
2. on a string ref, it attempts variable-references.go:ResolveVarRefFromParentModuleCall(varRef string) to resolve it
3. if the type of the var ends up being string it attempts resolution again forever This is the first fix - now it returns the original string passed in if it loops
4. if it's // some other reference then we go to variable-references.go:ResolveVarRef(varRef string)
5. if the type of the var ends up being string it attempts resolution again forever This is the second fix - now it returns the original string passed in if it loops

I guess my confusion arrives in two places:

• What do you mean by does not resolve variables in the parent module call. - am I missing somewhere the variable needs to be checked again?
• Does Terrascan account for the fact that some variables are simply not going to be resolvable because there's runtime variables in terraform? i.e. if I am passing in a .tfvars file when I run terraform, terrascan will not be able to see those variables, because terrascan does not support .tfvars files, etc.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[dinedal]

@kanchwala-yusuf Thanks for the merge!