Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Fix terraform_wrapper_module_for_each hook heredoc vars defaults #554

Merged
merged 3 commits into from
Aug 10, 2023
Merged

fix: Fix terraform_wrapper_module_for_each hook heredoc vars defaults #554

merged 3 commits into from
Aug 10, 2023

Conversation

ajax-ryzhyi-r
Copy link
Contributor

Put an x into the box if that apply:

  • This PR introduces breaking change.
  • This PR fixes a bug.
  • This PR adds new functionality.
  • This PR enhances existing functionality.

Description of your changes

Fixes #552 comments

If default variable values are written in heredoc style, the hook produces an error:

Terraform wrapper with for_each in module................................Failed
- hook id: terraform_wrapper_module_for_each
- exit code: 1

failed to build expression at the parse phase: failed to parse input: generated_by_buildExpression:21,7-7: Unterminated template string; No closing marker was found for the string.

This error occurs because the heredoc syntax requires the closing marker to be alone on its own line. This pull request adds a new line at the end for heredoc default values.

Also, this pull request addresses @yermulnik's comment in issue #552.

How can we test changes

Changes can be tested against any module that has heredoc default values for variables. For example, the terraform-aws-appsync module.

@ajax-ryzhyi-r ajax-ryzhyi-r mentioned this pull request Aug 10, 2023
Merged
4 tasks
@ajax-ryzhyi-r ajax-ryzhyi-r changed the title fix: Fix terraform_wrapper_module_for_each hook herdoc vars defaults fix: Fix terraform_wrapper_module_for_each hook heredoc vars defaults Aug 10, 2023
antonbabenko
antonbabenko requested changes Aug 10, 2023
View reviewed changes
Copy link
Owner

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good, and it works :) A slight improvement is missing.

hooks/terraform_wrapper_module_for_each.sh Outdated Show resolved Hide resolved
@antonbabenko antonbabenko merged commit 6fd4263 into antonbabenko:master Aug 10, 2023
4 checks passed
antonbabenko pushed a commit that referenced this pull request Aug 10, 2023
@semantic-release-bot
chore(release): version 1.81.2 [skip ci]
6cdea1b 
## [1.81.2](v1.81.1...v1.81.2) (2023-08-10)

### Bug Fixes

* Fix terraform_wrapper_module_for_each hook heredoc vars defaults ([#554](#554)) ([6fd4263](6fd4263))
@antonbabenko
Copy link
Owner

This PR is included in version 1.81.2 🎉

@antonbabenko antonbabenko mentioned this pull request Aug 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonbabenko antonbabenko antonbabenko approved these changes

@MaxymVlasov MaxymVlasov Awaiting requested review from MaxymVlasov MaxymVlasov is a code owner

@yermulnik yermulnik Awaiting requested review from yermulnik yermulnik is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ajax-ryzhyi-r @antonbabenko