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

feat: Add ip_address to bgp_peer_options in vpn_ha #100

Merged

Conversation

tnk-ysk
Copy link
Contributor

@tnk-ysk tnk-ysk commented Apr 19, 2023

Summary

Add ip_address field to bgp_peer_options to allow users to specify the ip_address of the google_compute_router_peer`.

References

@tnk-ysk tnk-ysk requested a review from a team as a code owner April 19, 2023 07:56
@google-cla
Copy link

google-cla bot commented Apr 19, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@tnk-ysk tnk-ysk changed the title Add ip_address to bgp_peer_options feat: Add ip_address to bgp_peer_options of vpn_ha Apr 19, 2023
@tnk-ysk tnk-ysk changed the title feat: Add ip_address to bgp_peer_options of vpn_ha feat: Add ip_address to bgp_peer_options in vpn_ha May 9, 2023
@imrannayer
Copy link
Collaborator

/gcbrun

@tnk-ysk tnk-ysk requested a review from imrannayer as a code owner July 5, 2023 17:15
@imrannayer
Copy link
Collaborator

/gcbrun

modules/vpn_ha/main.tf Outdated Show resolved Hide resolved
@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer imrannayer self-requested a review July 12, 2023 14:19
@imrannayer imrannayer merged commit ca72e53 into terraform-google-modules:master Jul 12, 2023
4 checks passed
@tnk-ysk tnk-ysk deleted the add-ip_address-to-vpn_ha branch July 12, 2023 15:08
@ivayloivanov388
Copy link

Please make this an optional variable. If you use the latest module without optional variable, then the module errors if you don't have it in your confirm
..........>
│ 39: backup-vpn = {
│ 40: bgp_peer = {
│ 41: address = var.backup_bgp_remote_ip
│ 42: asn = var.bgp_peer_asn
│ 43: }
│ 44: bgp_peer_options = {
│ 45: advertise_groups = null
│ 46: advertise_ip_ranges = var.advertised_routes
│ 47: advertise_mode = "CUSTOM"
│ 48: route_priority = 100
│ 49: }

...........

│ The given value is not suitable for module.vpn_ha_wa2.module.vpn_ha_wa2.var.tunnels declared at
│ .terraform/modules/vpn_ha_wa2.vpn_ha_wa2/modules/vpn_ha/variables.tf:90,1-19: element "main-vpn": attribute "bgp_peer_options":
│ attribute "ip_address" is required.

@tnk-ysk
Copy link
Contributor Author

tnk-ysk commented Jul 13, 2023

Optional attributes were added in terraform 1.3.0.
https://github.com/hashicorp/terraform/blob/v1.3/CHANGELOG.md#130-september-21-2022

but this module is meant for use with Terraform 0.12, That's why I didn't use optional.

This module is meant for use with Terraform 0.12. If you haven't [upgraded](https://www.terraform.io/upgrade-guides/0-12.html)

@imrannayer
Copy link
Collaborator

This module has optional attribute for some time. We need to update version information to reflect it and also make attributes optional if they are optional. Will create a separate Issue and PR.

Thanks

@imrannayer
Copy link
Collaborator

Please make this an optional variable. If you use the latest module without optional variable, then the module errors if you don't have it in your confirm ..........> │ 39: backup-vpn = { │ 40: bgp_peer = { │ 41: address = var.backup_bgp_remote_ip │ 42: asn = var.bgp_peer_asn │ 43: } │ 44: bgp_peer_options = { │ 45: advertise_groups = null │ 46: advertise_ip_ranges = var.advertised_routes │ 47: advertise_mode = "CUSTOM" │ 48: route_priority = 100 │ 49: }

...........

│ The given value is not suitable for module.vpn_ha_wa2.module.vpn_ha_wa2.var.tunnels declared at │ .terraform/modules/vpn_ha_wa2.vpn_ha_wa2/modules/vpn_ha/variables.tf:90,1-19: element "main-vpn": attribute "bgp_peer_options": │ attribute "ip_address" is required.

Can you set ip_address = null for the time being?

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.

3 participants