terraform/module: fix reference collection skipping

[radeksimko]

This is a typo copy paste bug which was introduced early with the introduction of these operations in #485

Attached test exhibits the behaviour.

This is blocker for #700 where we rely on reference origins & targets to always be parsed.

[dbanck]

LGTM (as far as I can tell)

Interesting behavior of txn.Insert.

It may be worth evaluating if we want to add tests for the other module operations at some point later.

[radeksimko]

@dbanck

Interesting behavior of txn.Insert.

If you're pointing out the data race, then that was caused by modifying the "live" record as opposed to making copy when updating, which doesn't have much to do with Insert itself, but rather the underlying data structures. Generally we store pointers in memdb, which means that if you ask for the same record twice, returned record will point to the same place in memory - which makes sense as we don't want to be making copy every time a record is read.

If you're pointing out the Insert method itself then it behaves more like "upsert" in some databases, which may be a little confusing, I admit. Unfortunately memdb API doesn't have a dedicated Update method (yet), but there are some TODOs in the codebase for that.

It may be worth evaluating if we want to add tests for the other module operations at some point later.

Yes, indeed - I think we should. I'm more wondering if there's any way we could refactor the logic such that it negates the need for all these constants and mappings between functions and operation names - I think that in itself just leaves too much room for error.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.