fix: avoid OOM and infinite loops in moduleInfo.load()

[finnag]

what

moduleInfo.load() now loads modules and dependencies without recursing, and only loads each dependency once.

why

moduleInfo.load() used to load modules and depedencies recursively, and due to some unfortunate circumstances it would usually create a very deep call stack where each load() called another load() to load the next one.

The terraform parsing uses enough stack space that this can become a problem, so for a slightly big project this sometimes caused atlantis to die from a stack overflow:

runtime: goroutine stack exceeds 1000000000-byte limit runtime: sp=0xc021478380 stack=[0xc021478000, 0xc041478000] fatal error: stack overflow
[...]
github.com/runatlantis/atlantis/server/events.moduleInfo.load(...)
	...atlantis/server/events/modules.go:108 +0x46b fp=0xc021478750 sp=0xc021478570 pc=0xfeaa6b
github.com/runatlantis/atlantis/server/events.moduleInfo.load(...)
	...atlantis/server/events/modules.go:108 +0x46b fp=0xc021478930 sp=0xc021478750 pc=0xfeaa6b
github.com/runatlantis/atlantis/server/events.moduleInfo.load(...)
	...atlantis/server/events/modules.go:108 +0x46b fp=0xc021478b10 sp=0xc021478930 pc=0xfeaa6b

... and so on, several hundred times.

tests

• I have tested my changes by make test-all

references

[GenPage]

@finnag Have we updated the tests in modules_test.go or do they cover this new change in functionality?

[glasser]

We just ran into this as well.

[jamengual]

@finnag do you have time to reply to the comment? Thanks.

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

[glasser]

Would love to see this get merged. I could take over if help is needed and @finnag is unavailable.

[jamengual]

if finnag do not have time, you can take it over.

…

On Sun, Dec 17, 2023, 2:48 p.m. David Glasser ***@***.***> wrote: Would love to see this get merged. I could take over if help is needed and @finnag <https://github.com/finnag> is unavailable. — Reply to this email directly, view it on GitHub <#3769 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ3ERAOL7UHSJOLAPCBC7LYJ5ZCHAVCNFSM6AAAAAA4ZT7P22VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZGMYTAOBXGI> . You are receiving this because your review was requested.Message ID: ***@***.***>

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

[glasser]

Haven't had a chance to take over yet

[finnag]

Had some time to look at atlantis again and rebased all my PRs to v0.30.0. We've run with this one for a year+. The diff with whitespace removed is pretty minimal - what kind of tests are required for this?

[X-Guardian]

Hi @finnag. Take a look at the current server\events\modules_test.go file and add some tests that cover your code change, i.e. test cases with multiple duplicate dependencies and verify that they are only loaded once.