-
-
Notifications
You must be signed in to change notification settings - Fork 545
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: Fix terraform_wrapper_module_for_each for when resource name contains 'variable' #573
fix: Fix terraform_wrapper_module_for_each for when resource name contains 'variable' #573
Conversation
@@ -321,14 +321,14 @@ EOF | |||
|
|||
# Get names of module variables in all terraform files | |||
# shellcheck disable=SC2207 | |||
module_vars=($(echo "$all_tf_content" | hcledit block list | { grep variable. | cut -d'.' -f 2 | sort || true; })) | |||
module_vars=($(echo "$all_tf_content" | hcledit block list | { grep "^\s*variable." | cut -d'.' -f 2 | sort || true; })) |
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.
module_vars=($(echo "$all_tf_content" | hcledit block list | { grep "^\s*variable." | cut -d'.' -f 2 | sort || true; })) | |
module_vars=($(echo "$all_tf_content" | hcledit block list | { grep "^variable." | cut -d'.' -f 2 | sort || true; })) |
This should be enough. I don't think there can/should be any space before the "variable".
Please apply the same fix on other lines in this PR.
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 don't think there can/should be any space before the "variable".
Leading spaces do not produce errors, trigger failures and are not otherwise disallowed explicitly. Hence should be considered allowed imho.
On the other end hcledit block list
seems to strip leading spaces, which means Anton's recommendation is correct =)
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 also would like to recommend to escape dot to not mix it with "any char" in this use case:
module_vars=($(echo "$all_tf_content" | hcledit block list | { grep "^\s*variable." | cut -d'.' -f 2 | sort || true; })) | |
module_vars=($(echo "$all_tf_content" | hcledit block list | { grep "^variable\." | cut -d'.' -f 2 | sort || true; })) |
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.
And as Anton mentioned: please apply the same to other code bits in this PR.
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.
Done! Please resolve if this looks sufficient. Thanks!
f2cf876
to
c57e68c
Compare
## [1.83.4](v1.83.3...v1.83.4) (2023-09-22) ### Bug Fixes * Fix terraform_wrapper_module_for_each for when resource name contains 'variable' ([#573](#573)) ([941177e](941177e))
This PR is included in version 1.83.4 🎉 |
Put an
x
into the box if that apply:Description of your changes
There are some providers with resources and data sources that have "variable" in their name, for example:
When using such resources,
terraform_wrapper_module_for_each
was adding these as variables to pass inwrappers/main.tf
which breaks the wrapperMy fix simply improves
grep
to ensure thatmodule_vars
(andmodule_outputs
andmodule_providers
) only find actual variables, outputs, and providers and NOT resources.How can we test changes
Use the examples provided in the documentation for env0_configuration_variable and try
terraform_wrapper_module_for_each
as-is today. You'll see that it adds these as variables to pass to the module when it shouldn't.Now test with my fix and you'll see that it fixes the issue.