fix: Fixed ordering issue in terraform_wrapper_module_for_each hook #565
Conversation
…apper_module_for_each hook
ajax-ryzhyi-r
requested review from
MaxymVlasov and
yermulnik
as code owners
|
|
||
| # Get names of module outputs in all terraform files | ||
| # shellcheck disable=SC2207 | ||
| module_outputs=($(echo "$all_tf_content" | hcledit block list | { grep output. | cut -d'.' -f 2 || true; })) | ||
| module_outputs=($(echo "$all_tf_content" | hcledit block list | { grep output. | cut -d'.' -f 2 | sort || true; })) | ||
|
|
||
| # Get names of module providers in all terraform files | ||
| module_providers=$(echo "$all_tf_content" | hcledit block list | { grep provider. || true; }) |
There was a problem hiding this comment.
@ajax-ryzhyi-r Would it make sense to add sorting to this var definition as well for consistency?
There was a problem hiding this comment.
@ajax-ryzhyi-r Would it make sense to add sorting to this var definition as well for consistency?
This variable is only used to check if the module contains provider definitions, so the ordering does not matter. However, for the sake of code consistency, sorting can be added here as well.
There was a problem hiding this comment.
@MaxymVlasov What do you think? I'd add sorting for this var value too.
There was a problem hiding this comment.
Good idea to add sort, but it is enough to have it just in module_vars as it is the only variable used in a loop when making a file.
Also, tflint may help with this. It would complain if variable blocks were defined in various places.
There was a problem hiding this comment.
Good idea to add
sort, but it is enough to have it just inmodule_varsas it is the only variable used in a loop when making a file.Also,
tflintmay help with this. It would complain ifvariableblocks were defined in various places.
Yes, you're right. The order of outputs does not matter too. Thank you for pointing that out.
I am aware that tflint can detect such cases, but we still have some legacy modules where this structure is expected 😢
…apper_module_for_each hook
antonbabenko
changed the title
## [1.83.1](v1.83.0...v1.83.1) (2023-09-04) ### Bug Fixes * Fixed ordering issue in terraform_wrapper_module_for_each hook ([#565](#565)) ([dc12be1](dc12be1))
|
This PR is included in version 1.83.1 🎉 |
Put an x into the box if that apply:
Description of your changes
I have a module with variables defined in two
.tffiles. When I run theterraform_wrapper_module_for_each hooklocally on macOS, no changes are shown. However, when the same hooks run against the same module by GitHub Actions on Ubuntu, a difference in the order of the wrapper module input variables is shown. After researching the root cause of this behavior, I discovered that thefindutility returns files in a different order on macOS and Ubuntu because these OS use different virtual file systems:https://unix.stackexchange.com/questions/13451/what-is-the-directory-order-of-files-in-a-directory-used-by-ls-u
To solve this issue, I added sorting for the outputs and variables before appending them to the wrapper module.
Note: These changes could cause updates to already generated wrappers.
How can we test changes
Run the hooks against module with the next structure in MacOS and Ubuntu: