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.

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

Enable users to control if firewall rules for health check are created #59

Conversation

wkrysmann
Copy link
Contributor

This module currently forces user to create firewall rules for health check. This PR changes that behavior adding new variable to control if firewall rules are created for health checks with default set to true and proper comment explaining the implications of setting it differently.

Motivation behind opening this configuration option is scenario where load-balancer is created in shared network and project creating it doesn't have access to manipulate firewall rules.

@comment-bot-dev
Copy link

comment-bot-dev commented Jul 14, 2021

Thanks for the PR! 🚀
✅ Lint checks have passed.

Mahsamtm
Mahsamtm previously approved these changes Jul 14, 2021
Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Thanks for adding this. Note you will also need to regenerate docs (make generate_docs).

variables.tf Outdated
type = bool
}

variable "create_healtcheck_firewall" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
variable "create_healtcheck_firewall" {
variable "create_health_check_firewall" {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the answer. I did that, but all of them are skipped. I don't see any changes to the PR afterwards:

Generating markdown docs with terraform-docs
Working in . ...
Success!
Skipping ./examples/minimal because README.md does not exist.
Working in ./examples/simple ...
Success!
Skipping ./test/fixtures/minimal because README.md does not exist.
Skipping ./test/setup because README.md does not exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't it because this module doesn't really have a table with variables in readme?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, looks like you're fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@morgante so you are good to give it 👍 ?

@morgante morgante merged commit 9206a3c into terraform-google-modules:master Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants